opp: Implement dev_pm_opp_set_opp()
Subsystems (12)
Viresh Kumar
maintainercover opp: Implement dev_pm_opp_set_opp()
Hello,
This patchset implements a new API dev_pm_opp_set_opp(), which configures the devices represented by an opp table to a particular opp. The opp core supports a wide variety of devices now, some of them can change frequency and other properties (like CPUs), while others can just change their pstates or regulators (like power domains) and then there are others which can change their bandwidth as well (interconnects). Instead of having separate implementations for all of them, where all will eventually lack something or the other, lets try to implement a common solution for everyone. This takes care of setting regulators, bw, required opps, etc for all device types.
Dmitry, please go ahead and try this series. This is based of opp tree's linux-next branch.
Sibi, since you added dev_pm_opp_set_bw() earlier, it would be good if you can give this a try. In case this breaks anything for you.
I have already tested this on hikey board for CPU devices.
To get this tested better and as early as possible, I have pushed it here:
git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/linux-next
This will be part of linux-next tomorrow.
Note, all the patches need to go through OPP tree here. Please provide your Acks for platform specific bits.
--
Viresh
Viresh Kumar (13):
opp: Rename _opp_set_rate_zero()
opp: No need to check clk for errors
opp: Keep track of currently programmed OPP
opp: Split _set_opp() out of dev_pm_opp_set_rate()
opp: Allow _set_opp() to work for non-freq devices
opp: Allow _generic_set_opp_regulator() to work for non-freq devices
opp: Allow _generic_set_opp_clk_only() to work for non-freq devices
opp: Update parameters of _set_opp_custom()
opp: Implement dev_pm_opp_set_opp()
cpufreq: qcom: Migrate to dev_pm_opp_set_opp()
devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
drm: msm: Migrate to dev_pm_opp_set_opp()
opp: Remove dev_pm_opp_set_bw()
drivers/cpufreq/qcom-cpufreq-hw.c | 2 +- drivers/devfreq/tegra30-devfreq.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 8 +- drivers/opp/core.c | 314 ++++++++++++++------------ drivers/opp/opp.h | 2 + include/linux/pm_opp.h | 6 +- 6 files changed, 184 insertions(+), 150 deletions(-)
Signature · 1 line
2.25.0.rc1.19.g042ed3e048af
Viresh Kumar
maintainerPATCH 1/13 opp: Rename _opp_set_rate_zero()
This routine has nothing to do with frequency, it just disables all the resources previously enabled. Rename it to match its purpose.
--- drivers/opp/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/opp/core.c +2 -2
@@ -943,7 +943,7 @@ int dev_pm_opp_set_bw(struct device *dev, struct dev_pm_opp *opp) }EXPORT_SYMBOL_GPL (dev_pm_opp_set_bw); -static int _opp_set_rate_zero (struct device *dev,struct opp_table *opp_table) +static int _disable_opp_table (struct device *dev,struct opp_table *opp_table) {int ret;
@@ -997,7 +997,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) }if (unlikely (!target_freq)) { - ret =_opp_set_rate_zero (dev, opp_table); + ret =_disable_opp_table (dev, opp_table); goto put_opp_table; }
Signature · 1 line
2.25.0.rc1.19.g042ed3e048af
Viresh Kumar
maintainerPATCH 2/13 opp: No need to check clk for errors
In reply to Viresh KumarClock is not optional for users who call into dev_pm_opp_set_rate(). Remove the unnecessary checks.
While at it also drop the local variable for clk and use opp_table->clk instead.
--- drivers/opp/core.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-)
drivers/opp/core.c +4 -13
@@ -987,7 +987,6 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)struct opp_table *opp_table;unsigned long freq, old_freq, temp_freq;struct dev_pm_opp *old_opp, *opp; -struct clk *clk;int ret; opp_table =_find_opp_table (dev);
@@ -1001,19 +1000,11 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) goto put_opp_table; } - clk = opp_table->clk; -if (IS_ERR (clk)) { -dev_err (dev,"%s: No clock available for the device\n" , - __func__); - ret =PTR_ERR (clk); - goto put_opp_table; - } - - freq =clk_round_rate (clk, target_freq); + freq =clk_round_rate (opp_table->clk, target_freq);if ((long )freq <=0 ) freq = target_freq; - old_freq =clk_get_rate (clk); + old_freq =clk_get_rate (opp_table->clk);/* Return early if nothing to do */ if (opp_table->enabled && old_freq == freq) {
@@ -1031,7 +1022,7 @@ 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, clk, freq); + ret =_generic_set_opp_clk_only (dev, opp_table->clk, freq); goto put_opp_table; }
@@ -1071,7 +1062,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) opp->supplies); }else {/* Only frequency scaling */ - ret =_generic_set_opp_clk_only (dev, clk, freq); + ret =_generic_set_opp_clk_only (dev, opp_table->clk, freq); }/* Scaling down? Configure required OPPs after frequency */
Signature · 1 line
2.25.0.rc1.19.g042ed3e048af
Viresh Kumar
maintainerPATCH 3/13 opp: Keep track of currently programmed OPP
In reply to Viresh KumarThe dev_pm_opp_set_rate() helper needs to know the currently programmed OPP to make few decisions and currently we try to find it on every invocation of this routine.
Lets start keeping track of the current_opp programmed for the devices of the opp table, that will be quite useful going forward.
If we fail to find the current OPP, we pick the first one available in the list, as the list is in ascending order of frequencies, level, or bandwidth and that's the best guess we can make anyway.
Note that we used to do the frequency comparison a bit early in dev_pm_opp_set_rate() previously, and now instead we check the target opp, which shall be more accurate anyway.
We need to make sure that current_opp's memory doesn't get freed while it is being used and so we keep a reference of it until the time it is used.
Now that current_opp will always be set, we can drop some unnecessary checks as well.
--- drivers/opp/core.c | 83 +++++++++++++++++++++++++++++----------------- drivers/opp/opp.h | 2 ++ 2 files changed, 55 insertions(+), 30 deletions(-)
drivers/opp/core.c +53 -30
@@ -788,8 +788,7 @@ static int _generic_set_opp_regulator(struct opp_table *opp_table, __func__, old_freq); restore_voltage:/* This shouldn't harm even if the voltages weren't updated earlier */ -if (old_supply) -_set_opp_voltage (dev, reg, old_supply); +_set_opp_voltage (dev, reg, old_supply);return ret; }
@@ -839,10 +838,7 @@ static int _set_opp_custom(const struct opp_table *opp_table, data->old_opp.rate = old_freq; size =sizeof (*old_supply) * opp_table->regulator_count; -if (!old_supply) -memset (data->old_opp.supplies,0 , size); -else -memcpy (data->old_opp.supplies, old_supply, size); +memcpy (data->old_opp.supplies, old_supply, size); data->new_opp.rate = freq;memcpy (data->new_opp.supplies, new_supply, size);
@@ -943,6 +939,31 @@ int dev_pm_opp_set_bw(struct device *dev, struct dev_pm_opp *opp) }EXPORT_SYMBOL_GPL (dev_pm_opp_set_bw); +static void _find_current_opp (struct device *dev,struct opp_table *opp_table) +{ +struct dev_pm_opp *opp =ERR_PTR (-ENODEV ); +unsigned long freq; + +if (!IS_ERR (opp_table->clk)) { + freq =clk_get_rate (opp_table->clk); + opp =_find_freq_ceil (opp_table, &freq); + } + +/* +* Unable to find the current OPP ? Pick the first from the list since +* it is in ascending order, otherwise rest of the code will need to +* make special checks to validate current_opp. +*/ +if (IS_ERR (opp)) { +mutex_lock (&opp_table->lock); + opp = list_first_entry(&opp_table->opp_list, struct dev_pm_opp, node); +dev_pm_opp_get (opp); +mutex_unlock (&opp_table->lock); + } + + opp_table->current_opp = opp; +} +static int _disable_opp_table (struct device *dev,struct opp_table *opp_table) {int ret;
@@ -1004,16 +1025,6 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if ((long )freq <=0 ) freq = target_freq; - old_freq =clk_get_rate (opp_table->clk); - -/* Return early if nothing to do */ -if (opp_table->enabled && old_freq == freq) { -dev_dbg (dev,"%s: old/new frequencies (%lu Hz) are same, nothing to do\n" , - __func__, freq); - ret =0 ; - goto put_opp_table; - } -/* * For IO devices which require an OPP on some platforms/SoCs * while just needing to scale the clock on some others
@@ -1026,12 +1037,9 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) goto put_opp_table; } - temp_freq = old_freq; - old_opp =_find_freq_ceil (opp_table, &temp_freq); -if (IS_ERR (old_opp)) { -dev_err (dev,"%s: failed to find current OPP for freq %lu (%ld)\n" , - __func__, old_freq,PTR_ERR (old_opp)); - } +/* Find the currently set OPP if we don't know already */ +if (unlikely (!opp_table->current_opp)) +_find_current_opp (dev, opp_table); temp_freq = freq; opp =_find_freq_ceil (opp_table, &temp_freq);
@@ -1039,7 +1047,17 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) ret =PTR_ERR (opp);dev_err (dev,"%s: failed to find OPP for freq %lu (%d)\n" , __func__, freq, ret); - goto put_old_opp; + goto put_opp_table; + } + + old_opp = opp_table->current_opp; + old_freq = old_opp->rate; + +/* Return early if nothing to do */ +if (opp_table->enabled && old_opp == opp) { +dev_dbg (dev,"%s: OPPs are same, nothing to do\n" , __func__); + ret =0 ; + goto put_opp; } dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n", __func__,
@@ -1054,11 +1072,10 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if (opp_table->set_opp) { ret =_set_opp_custom (opp_table, dev, old_freq, freq, -IS_ERR (old_opp) ?NULL : old_opp->supplies, - opp->supplies); + old_opp->supplies, opp->supplies); }else if (opp_table->regulators) { ret =_generic_set_opp_regulator (opp_table, dev, old_freq, freq, -IS_ERR (old_opp) ?NULL : old_opp->supplies, + old_opp->supplies, opp->supplies); }else {/* Only frequency scaling */
@@ -1074,15 +1091,18 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if (!ret) { ret =_set_opp_bw (opp_table, opp, dev, false); -if (!ret) +if (!ret) { opp_table->enabled = true; +dev_pm_opp_put (old_opp); + +/* Make sure current_opp doesn't get freed */ +dev_pm_opp_get (opp); + opp_table->current_opp = opp; + } } put_opp:dev_pm_opp_put (opp); -put_old_opp: -if (!IS_ERR (old_opp)) -dev_pm_opp_put (old_opp); put_opp_table:dev_pm_opp_put_opp_table (opp_table);return ret;
@@ -1276,6 +1296,9 @@ static void _opp_table_kref_release(struct kref *kref)list_del (&opp_table->node);mutex_unlock (&opp_table_lock); +if (opp_table->current_opp) +dev_pm_opp_put (opp_table->current_opp); +_of_clear_opp_table (opp_table);/* Release clk */
drivers/opp/opp.h +2 -0
@@ -135,6 +135,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_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. * @genpd_virt_devs: List of virtual devices for multiple genpd support.
@@ -183,6 +184,7 @@ struct opp_table {unsigned int parsed_static_opps;enum opp_table_access shared_opp; +struct dev_pm_opp *current_opp;struct dev_pm_opp *suspend_opp;struct mutex genpd_virt_dev_lock;
Signature · 7 lines
2.25.0.rc1.19.g042ed3e048af _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Dmitry Osipenko
maintainerRe: [PATCH 03/13] opp: Keep track of currently programmed OPP
21.01.2021 14:17, Viresh Kumar пишет:
Quoted 4 lines
drivers/opp/core.c +0
-0@@ -1074,15 +1091,18 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if (!ret) { ret =_set_opp_bw (opp_table, opp, dev, false);
drivers/opp/core.c int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) · line 1102
Quoted 10 lines
drivers/opp/core.c +7
-1-if (!ret) +if (!ret) { opp_table->enabled = true; +dev_pm_opp_put (old_opp); + +/* Make sure current_opp doesn't get freed */ +dev_pm_opp_get (opp); + opp_table->current_opp = opp; + } }
I'm a bit surprised that _set_opp_bw() isn't used similarly to _set_opp_voltage() in _generic_set_opp_regulator().
I'd expect the BW requirement to be raised before the clock rate goes UP.
Viresh Kumar
maintainerRe: [PATCH 03/13] opp: Keep track of currently programmed OPP
Quoted 19 lines · Dmitry Osipenko
21.01.2021 14:17, Viresh Kumar пишет:
Quoted 4 lines
drivers/opp/core.c +0
-0@@ -1074,15 +1091,18 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if (!ret) { ret =_set_opp_bw (opp_table, opp, dev, false);drivers/opp/core.c int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) · line 1102
Quoted 10 lines
drivers/opp/core.c +7
-1-if (!ret) +if (!ret) { opp_table->enabled = true; +dev_pm_opp_put (old_opp); + +/* Make sure current_opp doesn't get freed */ +dev_pm_opp_get (opp); + opp_table->current_opp = opp; + } }I'm a bit surprised that
_set_opp_bw()isn't used similarly to_set_opp_voltage()in_generic_set_opp_regulator().
Quoted 1 line
I'd expect the BW requirement to be raised before the clock rate goes UP.
I remember discussing that earlier when this stuff came in, and this I believe is the reason for that.
We need to scale regulators before/after frequency because when we increase the frequency a regulator may _not_ be providing enough power to sustain that (even for a short while) and this may have undesired effects on the hardware and so it is important to prevent that malfunction.
In case of bandwidth such issues will not happen (AFAIK) and doing it just once is normally enough. It is just about allowing more data to be transmitted, and won't make the hardware behave badly.
Signature · 1 line
viresh
Dmitry Osipenko
maintainerRe: [PATCH 03/13] opp: Keep track of currently programmed OPP
22.01.2021 07:45, Viresh Kumar пишет:
Quoted 31 lines · Dmitry Osipenko
Quoted 19 lines
21.01.2021 14:17, Viresh Kumar пишет:
Quoted 4 lines
drivers/opp/core.c +0
-0@@ -1074,15 +1091,18 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if (!ret) { ret =_set_opp_bw (opp_table, opp, dev, false);drivers/opp/core.c int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) · line 1102
Quoted 10 lines
drivers/opp/core.c +7
-1-if (!ret) +if (!ret) { opp_table->enabled = true; +dev_pm_opp_put (old_opp); + +/* Make sure current_opp doesn't get freed */ +dev_pm_opp_get (opp); + opp_table->current_opp = opp; + } }I'm a bit surprised that
_set_opp_bw()isn't used similarly to_set_opp_voltage()in_generic_set_opp_regulator().Quoted 1 line
I'd expect the BW requirement to be raised before the clock rate goes UP.
I remember discussing that earlier when this stuff came in, and this I believe is the reason for that.
We need to scale regulators before/after frequency because when we increase the frequency a regulator may _not_ be providing enough power to sustain that (even for a short while) and this may have undesired effects on the hardware and so it is important to prevent that malfunction.
Quoted 4 lines
In case of bandwidth such issues will not happen (AFAIK) and doing it just once is normally enough. It is just about allowing more data to be transmitted, and won't make the hardware behave badly.
This may not be true for all kinds of hardware, a display controller is one example. If display's pixclock is raised before the memory bandwidth of the display's memory client, then display controller may get a memory underflow since it won't be able to fetch memory fast enough and it's not possible to pause data transmission to display panel, hence display panel may get out of sync and a full hardware reset will be needed in order to recover. At least this is the case for NVIDIA Tegra SoCs.
I guess it's not a real problem for any of OPP API users right now, but this is something to keep in mind.
Viresh Kumar
maintainerRe: [PATCH 03/13] opp: Keep track of currently programmed OPP
Quoted 7 lines · Dmitry Osipenko
This may not be true for all kinds of hardware, a display controller is one example. If display's pixclock is raised before the memory bandwidth of the display's memory client, then display controller may get a memory underflow since it won't be able to fetch memory fast enough and it's not possible to pause data transmission to display panel, hence display panel may get out of sync and a full hardware reset will be needed in order to recover. At least this is the case for NVIDIA Tegra SoCs.
Hmm, but I expected that the request for more data will only come after the opp-set-rate has finished and not in between. May be I am wrong. There is nothing wrong in doing it the regulator way if required.
Quoted 2 lines
I guess it's not a real problem for any of OPP API users right now, but this is something to keep in mind.
Sure, I am not against it. Just that we thought it isn't worth the code.
Signature · 1 line
viresh
Re: [PATCH 03/13] opp: Keep track of currently programmed OPP
In reply to Viresh KumarQuoted 31 lines · Viresh Kumar
Quoted 19 lines · Dmitry Osipenko
21.01.2021 14:17, Viresh Kumar пишет:
Quoted 4 lines
drivers/opp/core.c +0
-0@@ -1074,15 +1091,18 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if (!ret) { ret =_set_opp_bw (opp_table, opp, dev, false);drivers/opp/core.c int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) · line 1102
Quoted 10 lines
drivers/opp/core.c +7
-1-if (!ret) +if (!ret) { opp_table->enabled = true; +dev_pm_opp_put (old_opp); + +/* Make sure current_opp doesn't get freed */ +dev_pm_opp_get (opp); + opp_table->current_opp = opp; + } }I'm a bit surprised that
_set_opp_bw()isn't used similarly to_set_opp_voltage()in_generic_set_opp_regulator().Quoted 1 line
I'd expect the BW requirement to be raised before the clock rate goes UP.
I remember discussing that earlier when this stuff came in, and this I believe is the reason for that.
We need to scale regulators before/after frequency because when we increase the frequency a regulator may _not_ be providing enough power to sustain that (even for a short while) and this may have undesired effects on the hardware and so it is important to prevent that malfunction.
Quoted 4 lines
In case of bandwidth such issues will not happen (AFAIK) and doing it just once is normally enough. It is just about allowing more data to be transmitted, and won't make the hardware behave badly.
I agree with Dmitry. BW is a shared resource in a lot of architectures. Raising clk before increasing the bw can lead to a scenario where this client saturate the entire BW for whatever small duration it may be. This will impact the latency requirements of other clients.
-Akhil.
Viresh Kumar
maintainerRe: [PATCH 03/13] opp: Keep track of currently programmed OPP
Quoted 31 lines · Akhil P Oommen
Quoted 30 lines · Viresh Kumar
Quoted 18 lines · Dmitry Osipenko
21.01.2021 14:17, Viresh Kumar пишет:
drivers/opp/core.c int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) · line 1101
Quoted 13 lines
drivers/opp/core.c +7
-1@@ -1074,15 +1091,18 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if (!ret) { ret =_set_opp_bw (opp_table, opp, dev, false); -if (!ret) +if (!ret) { opp_table->enabled = true; +dev_pm_opp_put (old_opp); + +/* Make sure current_opp doesn't get freed */ +dev_pm_opp_get (opp); + opp_table->current_opp = opp; + } }I'm a bit surprised that
_set_opp_bw()isn't used similarly to_set_opp_voltage()in_generic_set_opp_regulator().Quoted 1 line
I'd expect the BW requirement to be raised before the clock rate goes UP.
I remember discussing that earlier when this stuff came in, and this I believe is the reason for that.
We need to scale regulators before/after frequency because when we increase the frequency a regulator may _not_ be providing enough power to sustain that (even for a short while) and this may have undesired effects on the hardware and so it is important to prevent that malfunction.
Quoted 8 lines
Quoted 4 lines
In case of bandwidth such issues will not happen (AFAIK) and doing it just once is normally enough. It is just about allowing more data to be transmitted, and won't make the hardware behave badly.
I agree with Dmitry. BW is a shared resource in a lot of architectures. Raising clk before increasing the bw can lead to a scenario where this client saturate the entire BW for whatever small duration it may be. This will impact the latency requirements of other clients.
I see. I will make the necessary changes then to fix it. Thanks guys.
Signature · 6 lines
viresh _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH 03/13] opp: Keep track of currently programmed OPP
In reply to Viresh KumarHi,
Quoted 15 lines · Viresh Kumar
The
dev_pm_opp_set_rate()helper needs to know the currently programmed OPP to make few decisions and currently we try to find it on every invocation of this routine.Lets start keeping track of the
current_oppprogrammed for the devices of the opp table, that will be quite useful going forward.If we fail to find the current OPP, we pick the first one available in the list, as the list is in ascending order of frequencies, level, or bandwidth and that's the best guess we can make anyway.
Note that we used to do the frequency comparison a bit early in
dev_pm_opp_set_rate()previously, and now instead we check the target opp, which shall be more accurate anyway.
Quoted 7 lines
We need to make sure that current_opp's memory doesn't get freed while it is being used and so we keep a reference of it until the time it is used.
Now that
current_oppwill always be set, we can drop some unnecessary checks as well.
I'm seeing some intermittent issues on Hikey960 after this patch, which reproduces as follows. I've used v5.13 for my testing.
root@buildroot:~# while true; do \
Quoted 12 lines
cd /sys/devices/system/cpu/cpufreq/; \
for policy in policy*; do \
cd /sys/devices/system/cpu/cpufreq/$policy; \
for freq in $(cat scaling_available_frequencies); do \
echo "userspace" > scaling_governor; \
sleep 1; \
echo $freq > scaling_setspeed; \
sleep 1; \
cpu="${policy: -1}"; \
mask="0x$(printf '%x\n' $((1 << $cpu)))"; \
sysev=$(~/taskset $mask ~/sysbench run --test=cpu --max-time=1 | grep "total number of events"); \
delivered=$(cat cpuinfo_cur_freq); \Quoted 8 lines
if [ "$freq" != "$delivered" ]; then \
echo "CPU$cpu - $freq setting failed: delivered $delivered, sysevents: $sysev"; \
else \
echo "CPU$cpu - $freq setting succeeded: delivered $delivered, sysevents: $sysev"; \
fi; \
echo "schedutil" > scaling_governor; \
sleep 1; \
done; done; done;CPU0 - 533000 setting succeeded: delivered 533000, sysevents: total number of events: 112
CPU0 - 999000 setting succeeded: delivered 999000, sysevents: total number of events: 209
CPU0 - 1402000 setting succeeded: delivered 1402000, sysevents: total number of events: 293
CPU0 - 1709000 setting succeeded: delivered 1709000, sysevents: total number of events: 357
CPU0 - 1844000 setting succeeded: delivered 1844000, sysevents: total number of events: 385
CPU4 - 903000 setting succeeded: delivered 903000, sysevents: total number of events: 249
CPU4 - 1421000 setting succeeded: delivered 1421000, sysevents: total number of events: 395
CPU4 - 1805000 setting succeeded: delivered 1805000, sysevents: total number of events: 502
CPU4 - 2112000 setting succeeded: delivered 2112000, sysevents: total number of events: 588
CPU4 - 2362000 setting succeeded: delivered 2362000, sysevents: total number of events: 657
This is an example of good behavior of changing frequencies. I'm putting this here first to show the sysbench results for each frequency, which is helping me make sure that the performance matches the new set frequency.
Notes: the change to the schedutil governor after each userspace driven frequency change was added because if the change is always to higher frequencies, the issue does not reproduce as easily; the sleep commands are added just to make sure the change gets the time to take effect.
From time to time (7/400 fail rate), I get the following failures:
CPU0 - 533000 setting failed: delivered 1402000, sysevents: total number of events: 293
CPU0 - 1402000 setting failed: delivered 533000, sysevents: total number of events: 112
CPU0 - 1402000 setting failed: delivered 533000, sysevents: total number of events: 112
CPU4 - 903000 setting failed: delivered 1421000, sysevents: total number of events: 394
CPU4 - 1805000 setting failed: delivered 903000, sysevents: total number of events: 249
CPU0 - 533000 setting failed: delivered 1402000, sysevents: total number of events: 293
CPU4 - 1805000 setting failed: delivered 903000, sysevents: total number of events: 251
Now comes the interesting part: what seems to fix it is a call to clk_get_rate(opp_table->clk) in _set_opp(), which is what basically happened before this patch, as _find_current_opp() was always called. I do not need to do anything with the returned frequency.
Therefore, by adding:
drivers/opp/core.c +8 -0
@@ -987,6 +987,7 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, {struct dev_pm_opp *old_opp;int scaling_down, ret; +unsigned long cur_freq;if (unlikely (!opp))return _disable_opp_table (dev, opp_table);
@@ -994,6 +995,13 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,/* Find the currently set OPP if we don't know already */ if (unlikely (!opp_table->current_opp))_find_current_opp (dev, opp_table); +else if (!IS_ERR (opp_table->clk)) { + cur_freq =clk_get_rate (opp_table->clk); +if (opp_table->current_rate != cur_freq) +pr_err ("OPP mismatch: %lu vs %lu!" , + opp_table->current_rate, + cur_freq); + } old_opp = opp_table->current_opp;
.. it does seem to solve the problem (no failures in 1000 frequency changes), although I do get a few OPP mismatch logs:
[ 667.495112] core: OPP mismatch: 1709000000 vs 1402000000! [ 7260.656154] core: OPP mismatch: 1421000000 vs 903000000! [ 7260.727717] core: OPP mismatch: 903000000 vs 1421000000! [ 8847.304323] core: OPP mismatch: 1709000000 vs 1402000000!
I'm not sure what is happening here so I'm hoping you guys have more knowledge to steer debugging in the right direction.
To be noted that I'm running an equivalent variant of this test on multiple boards, and none of them have issues, except for Hikey960.
Thanks,
Ionela.
Quoted 187 lines
Signed-off-by Viresh Kumar <[email protected]>--- drivers/opp/core.c | 83 +++++++++++++++++++++++++++++----------------- drivers/opp/opp.h | 2 ++ 2 files changed, 55 insertions(+), 30 deletions(-)drivers/opp/core.c +53
-30@@ -788,8 +788,7 @@ static int _generic_set_opp_regulator(struct opp_table *opp_table, __func__, old_freq); restore_voltage:/* This shouldn't harm even if the voltages weren't updated earlier */ -if (old_supply) -_set_opp_voltage (dev, reg, old_supply); +_set_opp_voltage (dev, reg, old_supply);return ret; }@@ -839,10 +838,7 @@ static int _set_opp_custom(const struct opp_table *opp_table, data->old_opp.rate = old_freq; size =sizeof (*old_supply) * opp_table->regulator_count; -if (!old_supply) -memset (data->old_opp.supplies,0 , size); -else -memcpy (data->old_opp.supplies, old_supply, size); +memcpy (data->old_opp.supplies, old_supply, size); data->new_opp.rate = freq;memcpy (data->new_opp.supplies, new_supply, size);@@ -943,6 +939,31 @@ int dev_pm_opp_set_bw(struct device *dev, struct dev_pm_opp *opp) }EXPORT_SYMBOL_GPL (dev_pm_opp_set_bw); +static void _find_current_opp (struct device *dev,struct opp_table *opp_table) +{ +struct dev_pm_opp *opp =ERR_PTR (-ENODEV ); +unsigned long freq; + +if (!IS_ERR (opp_table->clk)) { + freq =clk_get_rate (opp_table->clk); + opp =_find_freq_ceil (opp_table, &freq); + } + +/* +* Unable to find the current OPP ? Pick the first from the list since +* it is in ascending order, otherwise rest of the code will need to +* make special checks to validate current_opp. +*/ +if (IS_ERR (opp)) { +mutex_lock (&opp_table->lock); + opp = list_first_entry(&opp_table->opp_list, struct dev_pm_opp, node); +dev_pm_opp_get (opp); +mutex_unlock (&opp_table->lock); + } + + opp_table->current_opp = opp; +} +static int _disable_opp_table (struct device *dev,struct opp_table *opp_table) {int ret;@@ -1004,16 +1025,6 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if ((long )freq <=0 ) freq = target_freq; - old_freq =clk_get_rate (opp_table->clk); - -/* Return early if nothing to do */ -if (opp_table->enabled && old_freq == freq) { -dev_dbg (dev,"%s: old/new frequencies (%lu Hz) are same, nothing to do\n" , - __func__, freq); - ret =0 ; - goto put_opp_table; - } -/* * For IO devices which require an OPP on some platforms/SoCs * while just needing to scale the clock on some others @@ -1026,12 +1037,9 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) goto put_opp_table; } - temp_freq = old_freq; - old_opp =_find_freq_ceil (opp_table, &temp_freq); -if (IS_ERR (old_opp)) { -dev_err (dev,"%s: failed to find current OPP for freq %lu (%ld)\n" , - __func__, old_freq,PTR_ERR (old_opp)); - } +/* Find the currently set OPP if we don't know already */ +if (unlikely (!opp_table->current_opp)) +_find_current_opp (dev, opp_table); temp_freq = freq; opp =_find_freq_ceil (opp_table, &temp_freq);@@ -1039,7 +1047,17 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) ret =PTR_ERR (opp);dev_err (dev,"%s: failed to find OPP for freq %lu (%d)\n" , __func__, freq, ret); - goto put_old_opp; + goto put_opp_table; + } + + old_opp = opp_table->current_opp; + old_freq = old_opp->rate; + +/* Return early if nothing to do */ +if (opp_table->enabled && old_opp == opp) { +dev_dbg (dev,"%s: OPPs are same, nothing to do\n" , __func__); + ret =0 ; + goto put_opp; } dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n", __func__,@@ -1054,11 +1072,10 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if (opp_table->set_opp) { ret =_set_opp_custom (opp_table, dev, old_freq, freq, -IS_ERR (old_opp) ?NULL : old_opp->supplies, - opp->supplies); + old_opp->supplies, opp->supplies); }else if (opp_table->regulators) { ret =_generic_set_opp_regulator (opp_table, dev, old_freq, freq, -IS_ERR (old_opp) ?NULL : old_opp->supplies, + old_opp->supplies, opp->supplies); }else {/* Only frequency scaling */ @@ -1074,15 +1091,18 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if (!ret) { ret =_set_opp_bw (opp_table, opp, dev, false); -if (!ret) +if (!ret) { opp_table->enabled = true; +dev_pm_opp_put (old_opp); + +/* Make sure current_opp doesn't get freed */ +dev_pm_opp_get (opp); + opp_table->current_opp = opp; + } } put_opp:dev_pm_opp_put (opp); -put_old_opp: -if (!IS_ERR (old_opp)) -dev_pm_opp_put (old_opp); put_opp_table:dev_pm_opp_put_opp_table (opp_table);return ret;@@ -1276,6 +1296,9 @@ static void _opp_table_kref_release(struct kref *kref)list_del (&opp_table->node);mutex_unlock (&opp_table_lock); +if (opp_table->current_opp) +dev_pm_opp_put (opp_table->current_opp); +_of_clear_opp_table (opp_table);/* Release clk */ drivers/opp/opp.h +2
-0@@ -135,6 +135,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_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. * @genpd_virt_devs: List of virtual devices for multiple genpd support. @@ -183,6 +184,7 @@ struct opp_table {unsigned int parsed_static_opps;enum opp_table_access shared_opp; +struct dev_pm_opp *current_opp;struct dev_pm_opp *suspend_opp;struct mutex genpd_virt_dev_lock;Signature · 1 line
2.25.0.rc1.19.g042ed3e048af
Viresh Kumar
maintainerRe: [PATCH 03/13] opp: Keep track of currently programmed OPP
Quoted 4 lines · Ionela Voinescu
Now comes the interesting part: what seems to fix it is a call to
clk_get_rate(opp_table->clk)in_set_opp(), which is what basically happened before this patch, as_find_current_opp()was always called. I do not need to do anything with the returned frequency.
Wow, thanks for narrowing it down this far :)
I had a quick look and this is what I think is the problem here.
This platform uses mailbox API to send its frequency change requests to another processor. And the way it is written currently, I don't see any guarantee whatsoever which say
"once clk_set_rate() returns, the frequency would have already changed".
And this may exactly be the thing you are able to hit, luckily because of this patchset :)
As a quick way of checking if that is right or not, this may make it work:
drivers/mailbox/hi3660-mailbox.c +3 -0
@@ -201,6 +201,9 @@ static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)/* Trigger data transferring */ writel (BIT (mchan->ack_irq), base +MBOX_SEND_REG ); + +hi3660_mbox_check_state (chan); +return 0 ; }
-------------------------8<-------------------------
As a proper fix, something like this (not even compile tested) is required I believe as I don't see the clients would know if the transfer is over. Cc'ing mailbox guys to see what can be done.
drivers/clk/hisilicon/clk-hi3660-stub.c +1 -2
@@ -89,7 +89,6 @@ static int hi3660_stub_clk_set_rate(struct clk_hw *hw, unsigned long rate, stub_clk->msg[0], stub_clk->msg[1]);mbox_send_message (stub_clk_chan.mbox, stub_clk->msg); -mbox_client_txdone (stub_clk_chan.mbox,0 ); stub_clk->rate = rate;return 0 ;
@@ -131,7 +130,7 @@ static int hi3660_stub_clk_probe(struct platform_device *pdev)/* Use mailbox client without blocking */ stub_clk_chan.cl.dev = dev; stub_clk_chan.cl.tx_done =NULL ; - stub_clk_chan.cl.tx_block = false; + stub_clk_chan.cl.tx_block = true; stub_clk_chan.cl.knows_txdone = false;/* Allocate mailbox channel */
drivers/mailbox/hi3660-mailbox.c +7 -9
@@ -1,5 +1,5 @@// SPDX-License-Identifier: GPL-2.0 -// Copyright (c) 2017-2018 HiSilicon Limited. +// Copyright (c) 2017-2018 Hisilicon Limited. // Copyright (c) 2017-2018 Linaro Limited. #include <linux/bitops.h>
@@ -83,7 +83,7 @@ static struct hi3660_mbox *to_hi3660_mbox(struct mbox_controller *mbox) return container_of(mbox, struct hi3660_mbox, controller); } -static int hi3660_mbox_check_state (struct mbox_chan *chan) +static bool hi3660_mbox_last_tx_done (struct mbox_chan *chan) {unsigned long ch = (unsigned long )chan->con_priv;struct hi3660_mbox *mbox =to_hi3660_mbox (chan->mbox);
@@ -94,20 +94,20 @@ static int hi3660_mbox_check_state(struct mbox_chan *chan)/* Mailbox is ready to use */ if (readl (base +MBOX_MODE_REG ) &MBOX_STATE_READY ) -return 0 ; +return true;/* Wait for acknowledge from remote */ ret =readx_poll_timeout_atomic (readl, base +MBOX_MODE_REG , val, (val &MBOX_STATE_ACK ),1000 ,300000 );if (ret) {dev_err (mbox->dev,"%s: timeout for receiving ack\n" , __func__); -return ret; +return false; }/* clear ack state, mailbox will get back to ready state */ writel (BIT (mchan->ack_irq), base +MBOX_ICLR_REG ); -return 0 ; +return true; }static int hi3660_mbox_unlock (struct mbox_chan *chan)
@@ -182,10 +182,6 @@ static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)unsigned int i;int ret; - ret =hi3660_mbox_check_state (chan); -if (ret) -return ret; -/* Clear mask for destination interrupt */ writel_relaxed (~BIT (mchan->dst_irq), base +MBOX_IMASK_REG );
@@ -207,6 +203,7 @@ static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)static const struct mbox_chan_ops hi3660_mbox_ops = { .startup = hi3660_mbox_startup, .send_data = hi3660_mbox_send_data, + .last_tx_done = hi3660_mbox_last_tx_done, }; static struct mbox_chan *hi3660_mbox_xlate(struct mbox_controller *controller,
@@ -259,6 +256,7 @@ static int hi3660_mbox_probe(struct platform_device *pdev) mbox->controller.num_chans =MBOX_CHAN_MAX ; mbox->controller.ops = &hi3660_mbox_ops; mbox->controller.of_xlate = hi3660_mbox_xlate; + mbox->controller.txdone_poll = true;/* Initialize mailbox channel data */ chan = mbox->chan;
Signature · 1 line
viresh
Re: [PATCH 03/13] opp: Keep track of currently programmed OPP
Hi Viresh,
Quoted 8 lines · Viresh Kumar
Quoted 4 lines · Ionela Voinescu
Now comes the interesting part: what seems to fix it is a call to
clk_get_rate(opp_table->clk)in_set_opp(), which is what basically happened before this patch, as_find_current_opp()was always called. I do not need to do anything with the returned frequency.Wow, thanks for narrowing it down this far :)
Quoted 8 lines
I had a quick look and this is what I think is the problem here.
This platform uses mailbox API to send its frequency change requests to another processor. And the way it is written currently, I don't see any guarantee whatsoever which say
"once
clk_set_rate()returns, the frequency would have already changed".
I think what was strange to me was that the frequency never seems to change, there isn't just a delay in the new frequency taking effect, as I would expect in these cases. Or if there is a delay, that's quite large
- at least a second.
Quoted 10 lines
And this may exactly be the thing you are able to hit, luckily because of this patchset :)
As a quick way of checking if that is right or not, this may make it work:
drivers/mailbox/hi3660-mailbox.c +0
-0
drivers/mailbox/hi3660-mailbox.c static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg) · line 206
Quoted 6 lines
drivers/mailbox/hi3660-mailbox.c +3
-0@@ -201,6 +201,9 @@ static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)/* Trigger data transferring */ writel (BIT (mchan->ack_irq), base +MBOX_SEND_REG ); + +hi3660_mbox_check_state (chan); +
I gave this a try an it does work for me.
Quoted 9 lines
drivers/mailbox/hi3660-mailbox.c +0
-1return 0 ; } -------------------------8<-------------------------As a proper fix, something like this (not even compile tested) is required I believe as I don't see the clients would know if the transfer is over. Cc'ing mailbox guys to see what can be done.
I'll give this a try as well when there is consensus. I might even try to review it, if the time allows.
Many thanks,
Ionela.
Quoted 95 lines
drivers/clk/hisilicon/clk-hi3660-stub.c +1
-2@@ -89,7 +89,6 @@ static int hi3660_stub_clk_set_rate(struct clk_hw *hw, unsigned long rate, stub_clk->msg[0], stub_clk->msg[1]);mbox_send_message (stub_clk_chan.mbox, stub_clk->msg); -mbox_client_txdone (stub_clk_chan.mbox,0 ); stub_clk->rate = rate;return 0 ;@@ -131,7 +130,7 @@ static int hi3660_stub_clk_probe(struct platform_device *pdev)/* Use mailbox client without blocking */ stub_clk_chan.cl.dev = dev; stub_clk_chan.cl.tx_done =NULL ; - stub_clk_chan.cl.tx_block = false; + stub_clk_chan.cl.tx_block = true; stub_clk_chan.cl.knows_txdone = false;/* Allocate mailbox channel */ drivers/mailbox/hi3660-mailbox.c +7
-9@@ -1,5 +1,5 @@// SPDX-License-Identifier: GPL-2.0 -// Copyright (c) 2017-2018 HiSilicon Limited. +// Copyright (c) 2017-2018 Hisilicon Limited. // Copyright (c) 2017-2018 Linaro Limited. #include <linux/bitops.h> @@ -83,7 +83,7 @@ static struct hi3660_mbox *to_hi3660_mbox(struct mbox_controller *mbox) return container_of(mbox, struct hi3660_mbox, controller); } -static int hi3660_mbox_check_state (struct mbox_chan *chan) +static bool hi3660_mbox_last_tx_done (struct mbox_chan *chan) {unsigned long ch = (unsigned long )chan->con_priv;struct hi3660_mbox *mbox =to_hi3660_mbox (chan->mbox);@@ -94,20 +94,20 @@ static int hi3660_mbox_check_state(struct mbox_chan *chan)/* Mailbox is ready to use */ if (readl (base +MBOX_MODE_REG ) &MBOX_STATE_READY ) -return 0 ; +return true;/* Wait for acknowledge from remote */ ret =readx_poll_timeout_atomic (readl, base +MBOX_MODE_REG , val, (val &MBOX_STATE_ACK ),1000 ,300000 );if (ret) {dev_err (mbox->dev,"%s: timeout for receiving ack\n" , __func__); -return ret; +return false; }/* clear ack state, mailbox will get back to ready state */ writel (BIT (mchan->ack_irq), base +MBOX_ICLR_REG ); -return 0 ; +return true; }static int hi3660_mbox_unlock (struct mbox_chan *chan)@@ -182,10 +182,6 @@ static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)unsigned int i;int ret; - ret =hi3660_mbox_check_state (chan); -if (ret) -return ret; -/* Clear mask for destination interrupt */ writel_relaxed (~BIT (mchan->dst_irq), base +MBOX_IMASK_REG );@@ -207,6 +203,7 @@ static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)static const struct mbox_chan_ops hi3660_mbox_ops = { .startup = hi3660_mbox_startup, .send_data = hi3660_mbox_send_data, + .last_tx_done = hi3660_mbox_last_tx_done, }; static struct mbox_chan *hi3660_mbox_xlate(struct mbox_controller *controller,@@ -259,6 +256,7 @@ static int hi3660_mbox_probe(struct platform_device *pdev) mbox->controller.num_chans =MBOX_CHAN_MAX ; mbox->controller.ops = &hi3660_mbox_ops; mbox->controller.of_xlate = hi3660_mbox_xlate; + mbox->controller.txdone_poll = true;/* Initialize mailbox channel data */ chan = mbox->chan;Signature · 2 lines
-- viresh
Viresh Kumar
maintainerRe: [PATCH 03/13] opp: Keep track of currently programmed OPP
Quoted 15 lines · Ionela Voinescu
Quoted 14 lines · Viresh Kumar
Quoted 4 lines · Ionela Voinescu
Now comes the interesting part: what seems to fix it is a call to
clk_get_rate(opp_table->clk)in_set_opp(), which is what basically happened before this patch, as_find_current_opp()was always called. I do not need to do anything with the returned frequency.Wow, thanks for narrowing it down this far :)
I had a quick look and this is what I think is the problem here.
This platform uses mailbox API to send its frequency change requests to another processor. And the way it is written currently, I don't see any guarantee whatsoever which say
Quoted 7 lines
Quoted 2 lines
"once
clk_set_rate()returns, the frequency would have already changed".I think what was strange to me was that the frequency never seems to change, there isn't just a delay in the new frequency taking effect, as I would expect in these cases. Or if there is a delay, that's quite large
- at least a second.
No idea on what the firmware is doing behind the scene :)
Quoted 11 lines
Quoted 11 lines
And this may exactly be the thing you are able to hit, luckily because of this patchset :)
As a quick way of checking if that is right or not, this may make it work:
drivers/mailbox/hi3660-mailbox.c +0
-0@@ -201,6 +201,9 @@ static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)
Quoted 7 lines
drivers/mailbox/hi3660-mailbox.c static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg) · line 206
Quoted 5 lines
drivers/mailbox/hi3660-mailbox.c +3
-0/* Trigger data transferring */ writel (BIT (mchan->ack_irq), base +MBOX_SEND_REG ); + +hi3660_mbox_check_state (chan); +I gave this a try an it does work for me.
Good, so that kind of proves what I was suspecting. The mailbox driver looks buggy here.
Quoted 9 lines
Quoted 6 lines
drivers/mailbox/hi3660-mailbox.c +0
-1-------------------------8<-------------------------As a proper fix, something like this (not even compile tested) is required I believe as I don't see the clients would know if the transfer is over. Cc'ing mailbox guys to see what can be done.
I'll give this a try as well when there is consensus. I might even try to review it, if the time allows.
Sure, lets see what the platform guys think about this first.
Kevin, Kaihua ?
Signature · 1 line
viresh
Viresh Kumar
maintainerPATCH 4/13 opp: Split _set_opp() out of dev_pm_opp_set_rate()
In reply to Viresh KumarThe _set_opp() helper will be used for devices which don't change their frequency (like power domains, etc.) later on, prepare for that by breaking the generic part out of dev_pm_opp_set_rate().
--- drivers/opp/core.c | 126 +++++++++++++++++++++++++-------------------- 1 file changed, 71 insertions(+), 55 deletions(-)
drivers/opp/core.c +71 -55
@@ -992,72 +992,27 @@ static int _disable_opp_table(struct device *dev, struct opp_table *opp_table)return ret; } -/** -* dev_pm_opp_set_rate() - Configure new OPP based on frequency -* @dev: device for which we do this operation -* @target_freq: frequency to achieve -* -* This configures the power-supplies to the levels specified by the OPP -* corresponding to the target_freq, and programs the clock to a value <= -* target_freq, as rounded by clk_round_rate(). Device wanting to run at fmax -* provided by the opp, should have already rounded to the target OPP's -* frequency. -*/ -int dev_pm_opp_set_rate (struct device *dev,unsigned long target_freq) +static int _set_opp (struct device *dev,struct opp_table *opp_table, +struct dev_pm_opp *opp,unsigned long freq) { -struct opp_table *opp_table; -unsigned long freq, old_freq, temp_freq; -struct dev_pm_opp *old_opp, *opp; +struct dev_pm_opp *old_opp; +unsigned long old_freq;int ret; - opp_table =_find_opp_table (dev); -if (IS_ERR (opp_table)) { -dev_err (dev,"%s: device opp doesn't exist\n" , __func__); -return PTR_ERR (opp_table); - } - -if (unlikely (!target_freq)) { - ret =_disable_opp_table (dev, opp_table); - goto put_opp_table; - } - - freq =clk_round_rate (opp_table->clk, target_freq); -if ((long )freq <=0 ) - freq = target_freq; - -/* -* For IO devices which require an OPP on some platforms/SoCs -* while just needing to scale the clock on some others -* we look for empty OPP tables with just a clock handle and -* scale only the clk. This makes dev_pm_opp_set_rate() -* equivalent to a clk_set_rate() -*/ -if (!_get_opp_count (opp_table)) { - ret =_generic_set_opp_clk_only (dev, opp_table->clk, freq); - goto put_opp_table; - } +if (unlikely (!opp)) +return _disable_opp_table (dev, opp_table);/* Find the currently set OPP if we don't know already */ if (unlikely (!opp_table->current_opp))_find_current_opp (dev, opp_table); - temp_freq = freq; - opp =_find_freq_ceil (opp_table, &temp_freq); -if (IS_ERR (opp)) { - ret =PTR_ERR (opp); -dev_err (dev,"%s: failed to find OPP for freq %lu (%d)\n" , - __func__, freq, ret); - goto put_opp_table; - } - old_opp = opp_table->current_opp; old_freq = old_opp->rate;/* Return early if nothing to do */ if (opp_table->enabled && old_opp == opp) {dev_dbg (dev,"%s: OPPs are same, nothing to do\n" , __func__); - ret =0 ; - goto put_opp; +return 0 ; } dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n", __func__,
@@ -1067,7 +1022,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)if (freq >= old_freq) { ret =_set_required_opps (dev, opp_table, opp, true);if (ret) - goto put_opp; +return ret; }if (opp_table->set_opp) {
@@ -1101,8 +1056,69 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) } } -put_opp: -dev_pm_opp_put (opp); +return ret; +} + +/** +* dev_pm_opp_set_rate() - Configure new OPP based on frequency +* @dev: device for which we do this operation +* @target_freq: frequency to achieve +* +* This configures the power-supplies to the levels specified by the OPP +* corresponding to the target_freq, and programs the clock to a value <= +* target_freq, as rounded by clk_round_rate(). Device wanting to run at fmax +* provided by the opp, should have already rounded to the target OPP's +* frequency. +*/ +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 ; +int ret; + + opp_table =_find_opp_table (dev); +if (IS_ERR (opp_table)) { +dev_err (dev,"%s: device's opp table doesn't exist\n" , __func__); +return PTR_ERR (opp_table); + } + +if (target_freq) { +/* +* For IO devices which require an OPP on some platforms/SoCs +* while just needing to scale the clock on some others +* we look for empty OPP tables with just a clock handle and +* scale only the clk. This makes dev_pm_opp_set_rate() +* equivalent to a clk_set_rate() +*/ +if (!_get_opp_count (opp_table)) { + ret =_generic_set_opp_clk_only (dev, opp_table->clk, target_freq); + goto put_opp_table; + } + + freq =clk_round_rate (opp_table->clk, target_freq); +if ((long )freq <=0 ) + freq = target_freq; + +/* +* The clock driver may support finer resolution of the +* frequencies than the OPP table, don't update the frequency we +* pass to clk_set_rate() here. +*/ + temp_freq = freq; + opp =_find_freq_ceil (opp_table, &temp_freq); +if (IS_ERR (opp)) { + ret =PTR_ERR (opp); +dev_err (dev,"%s: failed to find OPP for freq %lu (%d)\n" , + __func__, freq, ret); + goto put_opp_table; + } + } + + ret =_set_opp (dev, opp_table, opp, freq); + +if (opp) +dev_pm_opp_put (opp); put_opp_table:dev_pm_opp_put_opp_table (opp_table);return ret;
Signature · 7 lines
2.25.0.rc1.19.g042ed3e048af _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Viresh Kumar
maintainerPATCH 5/13 opp: Allow _set_opp() to work for non-freq devices
In reply to Viresh KumarThe _set_opp() helper will be used for devices which don't change frequency (like power domains, etc.) later on, prepare for that by not relying on frequency for making decisions here.
While at it, also update the debug print to contain all relevant information.
--- drivers/opp/core.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
drivers/opp/core.c +11 -5
@@ -997,7 +997,7 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, {struct dev_pm_opp *old_opp;unsigned long old_freq; -int ret; +int scaling_down, ret;if (unlikely (!opp))return _disable_opp_table (dev, opp_table);
@@ -1015,11 +1015,17 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,return 0 ; } -dev_dbg (dev,"%s: switching OPP: %lu Hz --> %lu Hz\n" , __func__, - old_freq, freq); +dev_dbg (dev,"%s: switching OPP: Freq %lu -> %lu Hz, Level %u -> %u, Bw %u -> %u\n" , + __func__, old_freq, freq, 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); +if (scaling_down ==-1 ) + scaling_down =0 ;/* Scaling up? Configure required OPPs before frequency */ -if (freq >= old_freq) { +if (!scaling_down) { ret =_set_required_opps (dev, opp_table, opp, true);if (ret)return ret;
@@ -1038,7 +1044,7 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, }/* Scaling down? Configure required OPPs after frequency */ -if (!ret && freq < old_freq) { +if (!ret && scaling_down) { ret =_set_required_opps (dev, opp_table, opp, false);if (ret)dev_err (dev,"Failed to set required opps: %d\n" , ret);
Signature · 7 lines
2.25.0.rc1.19.g042ed3e048af _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Viresh Kumar
maintainerPATCH 6/13 opp: Allow _generic_set_opp_regulator() to work for non-freq devices
In reply to Viresh KumarThe _generic_set_opp_regulator() helper will be used for devices which don't change frequency (like power domains, etc.) later on, prepare for that by not relying on frequency for making decisions here.
While at it, update its parameters to pass only what is necessary.
--- drivers/opp/core.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)
drivers/opp/core.c +12 -13
@@ -737,12 +737,12 @@ static inline int _generic_set_opp_clk_only(struct device *dev, struct clk *clk,static int _generic_set_opp_regulator (struct opp_table *opp_table,struct device *dev, -unsigned long old_freq, +struct dev_pm_opp *opp,unsigned long freq, -struct dev_pm_opp_supply *old_supply, -struct dev_pm_opp_supply *new_supply) +int scaling_down) {struct regulator *reg = opp_table->regulators[0 ]; +struct dev_pm_opp *old_opp = opp_table->current_opp;int ret;/* This function only supports single regulator per device */
@@ -752,8 +752,8 @@ static int _generic_set_opp_regulator(struct opp_table *opp_table, }/* Scaling up? Scale voltage before frequency */ -if (freq >= old_freq) { - ret =_set_opp_voltage (dev, reg, new_supply); +if (!scaling_down) { + ret =_set_opp_voltage (dev, reg, opp->supplies);if (ret) goto restore_voltage; }
@@ -764,8 +764,8 @@ static int _generic_set_opp_regulator(struct opp_table *opp_table, goto restore_voltage;/* Scaling down? Scale voltage after frequency */ -if (freq < old_freq) { - ret =_set_opp_voltage (dev, reg, new_supply); +if (scaling_down) { + ret =_set_opp_voltage (dev, reg, opp->supplies);if (ret) goto restore_freq; }
@@ -783,12 +783,12 @@ static int _generic_set_opp_regulator(struct opp_table *opp_table,return 0 ; restore_freq: -if (_generic_set_opp_clk_only (dev, opp_table->clk, old_freq)) +if (_generic_set_opp_clk_only (dev, opp_table->clk, old_opp->rate))dev_err (dev,"%s: failed to restore old-freq (%lu Hz)\n" , - __func__, old_freq); + __func__, old_opp->rate); restore_voltage:/* This shouldn't harm even if the voltages weren't updated earlier */ -_set_opp_voltage (dev, reg, old_supply); +_set_opp_voltage (dev, reg, old_opp->supplies);return ret; }
@@ -1035,9 +1035,8 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, ret =_set_opp_custom (opp_table, dev, old_freq, freq, old_opp->supplies, opp->supplies); }else if (opp_table->regulators) { - ret =_generic_set_opp_regulator (opp_table, dev, old_freq, freq, - old_opp->supplies, - opp->supplies); + ret =_generic_set_opp_regulator (opp_table, dev, opp, freq, + scaling_down); }else {/* Only frequency scaling */ ret =_generic_set_opp_clk_only (dev, opp_table->clk, freq);
Signature · 7 lines
2.25.0.rc1.19.g042ed3e048af _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Viresh Kumar
maintainerPATCH 7/13 opp: Allow _generic_set_opp_clk_only() to work for non-freq devices
In reply to Viresh KumarIn order to avoid conditional statements at the caller site, this patch updates _generic_set_opp_clk_only() to work for devices that don't change frequency (like power domains, etc.). Return 0 if the clk pointer passed to this routine is not valid.
--- drivers/opp/core.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/opp/core.c +4 -0
@@ -726,6 +726,10 @@ static inline int _generic_set_opp_clk_only(struct device *dev, struct clk *clk, {int ret; +/* We may reach here for devices which don't change frequency */ +if (unlikely (!clk)) +return 0 ; + ret =clk_set_rate (clk, freq);if (ret) { dev_err(dev, "%s: failed to set clock rate: %d\n", __func__,
Signature · 7 lines
2.25.0.rc1.19.g042ed3e048af _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Dmitry Osipenko
maintainerRe: [PATCH 07/13] opp: Allow _generic_set_opp_clk_only() to work for non-freq devices
21.01.2021 14:17, Viresh Kumar пишет:
Quoted 7 lines
In order to avoid conditional statements at the caller site, this patch updates
_generic_set_opp_clk_only()to work for devices that don't change frequency (like power domains, etc.). Return 0 if the clk pointer passed to this routine is not valid.Signed-off-by Viresh Kumar <[email protected]>---
...
Hello Viresh,
Thank you very much for yours effort! I gave a quick test to this series and instantly found one small issue in this patch.
drivers/opp/core.c static inline int _generic_set_opp_clk_only(struct device *dev, struct clk *clk, · line 730
Quoted 2 lines
drivers/opp/core.c +2
-0+/* We may reach here for devices which don't change frequency */ +if (unlikely (!clk))
I replaced dev_pm_opp_set_voltage() with dev_pm_opp_set_opp() in the Tegra PD driver and got a crash, which happens because the above line should be:
if (IS_ERR(clk))
The opp_table->clk is initialized to ERR_PTR(-ENOENT) if device doesn't have a clock, like a power domain device in my case.
Everything works good after fixing this patch. I'll keep testing and will be taking a closer look at the rest of the patches over this weekend.
For the record, here is a backtrace of the crash:
Unable to handle kernel NULL pointer dereference at virtual address 00000016
...
(clk_set_rate) from (_set_opp)
(_set_opp) from (dev_pm_opp_set_opp)
(dev_pm_opp_set_opp) from (tegra_genpd_set_performance_state) (tegra_genpd_set_performance_state) from (_genpd_set_performance_state) (_genpd_set_performance_state) from (dev_pm_genpd_set_performance_state) (dev_pm_genpd_set_performance_state) from (_set_required_opp) (_set_required_opp) from (_set_opp)
(_set_opp) from (dev_pm_opp_set_rate)
(dev_pm_opp_set_rate) from (host1x_runtime_resume) (host1x_runtime_resume) from (genpd_runtime_resume) (genpd_runtime_resume) from (__rpm_callback) (__rpm_callback) from (rpm_callback)
(rpm_callback) from (rpm_resume)
(rpm_resume) from (__pm_runtime_resume) (__pm_runtime_resume) from (host1x_probe)
(host1x_probe) from (platform_probe)
(platform_probe) from (really_probe)
(really_probe) from (driver_probe_device) (driver_probe_device) from (device_driver_attach) (device_driver_attach) from (__driver_attach) (__driver_attach) from (bus_for_each_dev)
(bus_for_each_dev) from (bus_add_driver)
(bus_add_driver) from (driver_register)
(driver_register) from (__platform_register_drivers) (__platform_register_drivers) from (host1x_module_init) (host1x_module_init) from (do_one_initcall) (do_one_initcall) from (kernel_init_freeable) (kernel_init_freeable) from (kernel_init)
Viresh Kumar
maintainerRe: [PATCH 07/13] opp: Allow _generic_set_opp_clk_only() to work for non-freq devices
Quoted 15 lines · Dmitry Osipenko
21.01.2021 14:17, Viresh Kumar пишет:
Quoted 7 lines
In order to avoid conditional statements at the caller site, this patch updates
_generic_set_opp_clk_only()to work for devices that don't change frequency (like power domains, etc.). Return 0 if the clk pointer passed to this routine is not valid.Signed-off-by Viresh Kumar <[email protected]>---
...
Hello Viresh,
Thank you very much for yours effort! I gave a quick test to this series and instantly found one small issue in this patch.
Quoted 8 lines
drivers/opp/core.c static inline int _generic_set_opp_clk_only(struct device *dev, struct clk *clk, · line 730
Quoted 2 lines
drivers/opp/core.c +2
-0+/* We may reach here for devices which don't change frequency */ +if (unlikely (!clk))I replaced
dev_pm_opp_set_voltage()withdev_pm_opp_set_opp()in the Tegra PD driver and got a crash, which happens because the above line should be:if (IS_ERR(clk))
Fixed, thanks.
Signature · 1 line
viresh
Dmitry Osipenko
maintainerRe: [PATCH 07/13] opp: Allow _generic_set_opp_clk_only() to work for non-freq devices
22.01.2021 07:35, Viresh Kumar пишет:
Quoted 23 lines · Dmitry Osipenko
Quoted 22 lines
21.01.2021 14:17, Viresh Kumar пишет:
Quoted 7 lines
In order to avoid conditional statements at the caller site, this patch updates
_generic_set_opp_clk_only()to work for devices that don't change frequency (like power domains, etc.). Return 0 if the clk pointer passed to this routine is not valid.Signed-off-by Viresh Kumar <[email protected]>---
...
Hello Viresh,
Thank you very much for yours effort! I gave a quick test to this series and instantly found one small issue in this patch.
drivers/opp/core.c static inline int _generic_set_opp_clk_only(struct device *dev, struct clk *clk, · line 730
Quoted 2 lines
drivers/opp/core.c +2
-0+/* We may reach here for devices which don't change frequency */ +if (unlikely (!clk))I replaced
dev_pm_opp_set_voltage()withdev_pm_opp_set_opp()in the Tegra PD driver and got a crash, which happens because the above line should be:
Quoted 4 lines
Quoted 1 line
if (IS_ERR(clk))Fixed, thanks.
Please remove unlikely() around IS_ERR(), it already has the unlikely().
https://elixir.bootlin.com/linux/v5.11-rc4/source/include/linux/err.h#L22
I'd also recommend to remove all the unlikely() from OPP code since it doesn't bring any value if not used in a very performance-critical code path. OPP core doesn't have such code paths. The [un]likely() only make code less readable and may result in a worse assembly.
Viresh Kumar
maintainerRe: [PATCH 07/13] opp: Allow _generic_set_opp_clk_only() to work for non-freq devices
Quoted 1 line · Dmitry Osipenko
Please remove
unlikely()aroundIS_ERR(), it already has theunlikely().
Right.
Quoted 6 lines
https://elixir.bootlin.com/linux/v5.11-rc4/source/include/linux/err.h#L22
I'd also recommend to remove all the
unlikely()from OPP code since it doesn't bring any value if not used in a very performance-critical code path. OPP core doesn't have such code paths. The [un]likely() only make code less readable and may result in a worse assembly.
The likely/unlikely() stuff is to optimize code, not necessarily the stuff in the hot path alone, therwise stuff like IS_ERR() would never have it. It surely does bring value by optimizing the code, surely the result isn't significant enough but that is fine, every effort counts.
AFAIK, if we are sure of path the code will almost always take, then we should rather use these and so I am inclined towards keeping them. Though I understand that using them may result in bad behavior (performance) if they fail.
Signature · 1 line
viresh
Viresh Kumar
maintainerPATCH 8/13 opp: Update parameters of _set_opp_custom()
In reply to Viresh KumarDrop the unnecessary parameters and follow the pattern from _generic_set_opp_regulator().
While at it, also remove the local variable old_freq.
--- drivers/opp/core.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
drivers/opp/core.c +9 -13
@@ -826,11 +826,10 @@ static int _set_opp_bw(const struct opp_table *opp_table, }static int _set_opp_custom (const struct opp_table *opp_table, -struct device *dev,unsigned long old_freq, -unsigned long freq, -struct dev_pm_opp_supply *old_supply, -struct dev_pm_opp_supply *new_supply) +struct device *dev,struct dev_pm_opp *opp, +unsigned long freq) { +struct dev_pm_opp *old_opp = opp_table->current_opp;struct dev_pm_set_opp_data *data;int size;
@@ -840,12 +839,12 @@ static int _set_opp_custom(const struct opp_table *opp_table, data->clk = opp_table->clk; data->dev = dev; - data->old_opp.rate = old_freq; - size =sizeof (*old_supply) * opp_table->regulator_count; -memcpy (data->old_opp.supplies, old_supply, size); + data->old_opp.rate = old_opp->rate; + size =sizeof (*old_opp->supplies) * opp_table->regulator_count; +memcpy (data->old_opp.supplies, old_opp->supplies, size); data->new_opp.rate = freq; -memcpy (data->new_opp.supplies, new_supply, size); +memcpy (data->new_opp.supplies, opp->supplies, size);return opp_table->set_opp (data); }
@@ -1000,7 +999,6 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, struct dev_pm_opp *opp, unsigned long freq) {struct dev_pm_opp *old_opp; -unsigned long old_freq;int scaling_down, ret;if (unlikely (!opp))
@@ -1011,7 +1009,6 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,_find_current_opp (dev, opp_table); old_opp = opp_table->current_opp; - old_freq = old_opp->rate;/* Return early if nothing to do */ if (opp_table->enabled && old_opp == opp) {
@@ -1020,7 +1017,7 @@ 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_freq, freq, old_opp->level, opp->level, + __func__, old_opp->rate, freq, old_opp->level, opp->level, old_opp->bandwidth ? old_opp->bandwidth[0 ].peak :0 , opp->bandwidth ? opp->bandwidth[0 ].peak :0 );
@@ -1036,8 +1033,7 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, }if (opp_table->set_opp) { - ret =_set_opp_custom (opp_table, dev, old_freq, freq, - old_opp->supplies, opp->supplies); + ret =_set_opp_custom (opp_table, dev, opp, freq); }else if (opp_table->regulators) { ret =_generic_set_opp_regulator (opp_table, dev, opp, freq, scaling_down);
Signature · 7 lines
2.25.0.rc1.19.g042ed3e048af _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Viresh Kumar
maintainerPATCH 9/13 opp: Implement dev_pm_opp_set_opp()
In reply to Viresh KumarThe new helper dev_pm_opp_set_opp() can be used for configuring the devices for a particular OPP and can be used by different type of devices, even the ones which don't change frequency (like power domains).
--- drivers/opp/core.c | 28 ++++++++++++++++++++++++++++ include/linux/pm_opp.h | 6 ++++++ 2 files changed, 34 insertions(+)
drivers/opp/core.c +28 -0
@@ -1130,6 +1130,34 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) }EXPORT_SYMBOL_GPL (dev_pm_opp_set_rate); +/** +* dev_pm_opp_set_opp() - Configure device for OPP +* @dev: device for which we do this operation +* @opp: OPP to set to +* +* This configures the device based on the properties of the OPP passed to this +* routine. +* +* Return: 0 on success, a negative error number otherwise. +*/ +int dev_pm_opp_set_opp (struct device *dev,struct dev_pm_opp *opp) +{ +struct opp_table *opp_table; +int ret; + + opp_table =_find_opp_table (dev); +if (IS_ERR (opp_table)) { +dev_err (dev,"%s: device opp doesn't exist\n" , __func__); +return PTR_ERR (opp_table); + } + + ret =_set_opp (dev, opp_table, opp, opp->rate); +dev_pm_opp_put_opp_table (opp_table); + +return ret; +} +EXPORT_SYMBOL_GPL (dev_pm_opp_set_opp); +/* OPP-dev Helpers */ static void _remove_opp_dev (struct opp_device *opp_dev,struct opp_table *opp_table)
include/linux/pm_opp.h +6 -0
@@ -158,6 +158,7 @@ void dev_pm_opp_detach_genpd(struct opp_table *opp_table);struct opp_table *devm_pm_opp_attach_genpd (struct device *dev,const char **names,struct device ***virt_devs);int dev_pm_opp_xlate_performance_state (struct opp_table *src_table,struct opp_table *dst_table,unsigned int pstate);int dev_pm_opp_set_rate (struct device *dev,unsigned long target_freq); +int dev_pm_opp_set_opp (struct device *dev,struct dev_pm_opp *opp);int dev_pm_opp_set_bw (struct device *dev,struct dev_pm_opp *opp);int dev_pm_opp_set_sharing_cpus (struct device *cpu_dev,const struct cpumask *cpumask);int dev_pm_opp_get_sharing_cpus (struct device *cpu_dev,struct cpumask *cpumask);
@@ -376,6 +377,11 @@ static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freturn -ENOTSUPP ; } +static inline int dev_pm_opp_set_opp (struct device *dev,struct dev_pm_opp *opp) +{ +return -ENOTSUPP ; +} +static inline int dev_pm_opp_set_bw (struct device *dev,struct dev_pm_opp *opp) {return -EOPNOTSUPP ;
Signature · 1 line
2.25.0.rc1.19.g042ed3e048af
Viresh Kumar
maintainerPATCH 10/13 cpufreq: qcom: Migrate to dev_pm_opp_set_opp()
In reply to Viresh Kumardev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should be used instead. Migrate to the new API.
--- drivers/cpufreq/qcom-cpufreq-hw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpufreq/qcom-cpufreq-hw.c +1 -1
@@ -54,7 +54,7 @@ static int qcom_cpufreq_set_bw(struct cpufreq_policy *policy,if (IS_ERR (opp))return PTR_ERR (opp); - ret =dev_pm_opp_set_bw (dev, opp); + ret =dev_pm_opp_set_opp (dev, opp);dev_pm_opp_put (opp);return ret; }
Signature · 1 line
2.25.0.rc1.19.g042ed3e048af
Viresh Kumar
maintainerPATCH 11/13 devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
In reply to Viresh Kumardev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should be used instead. Migrate to the new API.
--- drivers/devfreq/tegra30-devfreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/devfreq/tegra30-devfreq.c +1 -1
@@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,return PTR_ERR (opp); } - ret =dev_pm_opp_set_bw (dev, opp); + ret =dev_pm_opp_set_opp (dev, opp);dev_pm_opp_put (opp);return ret;
Signature · 1 line
2.25.0.rc1.19.g042ed3e048af
Dmitry Osipenko
maintainerRe: [PATCH 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
21.01.2021 14:17, Viresh Kumar пишет:
Quoted 14 lines
dev_pm_opp_set_bw()is getting removed anddev_pm_opp_set_opp()should be used instead. Migrate to the new API.Signed-off-by Viresh Kumar <[email protected]>--- drivers/devfreq/tegra30-devfreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/devfreq/tegra30-devfreq.c +0
-0
drivers/devfreq/tegra30-devfreq.c static int tegra_devfreq_target(struct device *dev, unsigned long *freq, · line 653
Quoted 8 lines
drivers/devfreq/tegra30-devfreq.c +1
-1@@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,return PTR_ERR (opp); } - ret =dev_pm_opp_set_bw (dev, opp); + ret =dev_pm_opp_set_opp (dev, opp);dev_pm_opp_put (opp);return ret;
This patch introduces a very serious change that needs to be fixed.
Now dev_pm_opp_set_opp() changes both clock rate and bandwidth, this is unacceptable for this driver because it shall not touch the clock rate.
I think dev_pm_opp_set_bw() can't be removed.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Viresh Kumar
maintainerRe: [PATCH 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
Quoted 26 lines · Dmitry Osipenko
21.01.2021 14:17, Viresh Kumar пишет:
Quoted 14 lines
dev_pm_opp_set_bw()is getting removed anddev_pm_opp_set_opp()should be used instead. Migrate to the new API.Signed-off-by Viresh Kumar <[email protected]>--- drivers/devfreq/tegra30-devfreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/devfreq/tegra30-devfreq.c +0
-0drivers/devfreq/tegra30-devfreq.c static int tegra_devfreq_target(struct device *dev, unsigned long *freq, · line 653
Quoted 8 lines
drivers/devfreq/tegra30-devfreq.c +1
-1@@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,return PTR_ERR (opp); } - ret =dev_pm_opp_set_bw (dev, opp); + ret =dev_pm_opp_set_opp (dev, opp);dev_pm_opp_put (opp);return ret;This patch introduces a very serious change that needs to be fixed.
Quoted 4 lines
Now
dev_pm_opp_set_opp()changes both clock rate and bandwidth, this is unacceptable for this driver because it shall not touch the clock rate.I think
dev_pm_opp_set_bw()can't be removed.
I am wondering here on what would be a better solution, do what you said or introduce another helper like dev_pm_opp_clear_clk(), which will make sure the OPP core doesn't play with device's clk.
Signature · 6 lines
viresh _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Dmitry Osipenko
maintainerRe: [PATCH 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
22.01.2021 09:26, Viresh Kumar пишет:
Quoted 27 lines · Dmitry Osipenko
Quoted 26 lines
21.01.2021 14:17, Viresh Kumar пишет:
Quoted 14 lines
dev_pm_opp_set_bw()is getting removed anddev_pm_opp_set_opp()should be used instead. Migrate to the new API.Signed-off-by Viresh Kumar <[email protected]>--- drivers/devfreq/tegra30-devfreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/devfreq/tegra30-devfreq.c +0
-0drivers/devfreq/tegra30-devfreq.c static int tegra_devfreq_target(struct device *dev, unsigned long *freq, · line 653
Quoted 8 lines
drivers/devfreq/tegra30-devfreq.c +1
-1@@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,return PTR_ERR (opp); } - ret =dev_pm_opp_set_bw (dev, opp); + ret =dev_pm_opp_set_opp (dev, opp);dev_pm_opp_put (opp);return ret;This patch introduces a very serious change that needs to be fixed.
Quoted 9 lines
Quoted 4 lines
Now
dev_pm_opp_set_opp()changes both clock rate and bandwidth, this is unacceptable for this driver because it shall not touch the clock rate.I think
dev_pm_opp_set_bw()can't be removed.I am wondering here on what would be a better solution, do what you said or introduce another helper like
dev_pm_opp_clear_clk(), which will make sure the OPP core doesn't play with device's clk.
Either way will work, but maybe keeping the dev_pm_opp_set_bw() is a bit more straightforward variant for now since it will avoid the code's changes and it's probably a bit more obvious variant for the OPP users.
Viresh Kumar
maintainerRe: [PATCH 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
Quoted 3 lines · Dmitry Osipenko
Either way will work, but maybe keeping the
dev_pm_opp_set_bw()is a bit more straightforward variant for now since it will avoid the code's changes and it's probably a bit more obvious variant for the OPP users.
The problem is it creates unnecessary paths which we need to support. For example, in case of bandwidth itself we may want to update regulator/pm domain/required OPPs and this should all be done for everyone. I really do not want to keep separate paths for such stuff, it makes it hard to maintain..
Signature · 1 line
viresh
Dmitry Osipenko
maintainerRe: [PATCH 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
25.01.2021 06:14, Viresh Kumar пишет:
Quoted 10 lines · Dmitry Osipenko
Quoted 3 lines
Either way will work, but maybe keeping the
dev_pm_opp_set_bw()is a bit more straightforward variant for now since it will avoid the code's changes and it's probably a bit more obvious variant for the OPP users.The problem is it creates unnecessary paths which we need to support. For example, in case of bandwidth itself we may want to update regulator/pm domain/required OPPs and this should all be done for everyone. I really do not want to keep separate paths for such stuff, it makes it hard to maintain..
Maybe we could add dev_pm_opp_of_add_table_without_clock(), at least that should be a bit nicer from a drivers perspective than having to care about dev_pm_opp_clear_clk(), IMO.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Viresh Kumar
maintainerPATCH v2 11/13 devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
In reply to Viresh Kumardev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should be used instead. Migrate to the new API.
We don't want the OPP core to manage the clk for this driver, migrate to dev_pm_opp_of_add_table_noclk() to make sure dev_pm_opp_set_opp() doesn't have any side effects.
---
Dmitry,
This is based over the patches sent here:
This should fix the problem you mentioned earlier. Will push this for linux-next unless you have any issues with it.
drivers/devfreq/tegra30-devfreq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/devfreq/tegra30-devfreq.c +2 -2
@@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,return PTR_ERR (opp); } - ret =dev_pm_opp_set_bw (dev, opp); + ret =dev_pm_opp_set_opp (dev, opp);dev_pm_opp_put (opp);return ret;
@@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)return err; } - err =dev_pm_opp_of_add_table (&pdev->dev); + err =dev_pm_opp_of_add_table_noclk (&pdev->dev);if (err) {dev_err (&pdev->dev,"Failed to add OPP table: %d\n" , err); goto put_hw;
Signature · 1 line
2.25.0.rc1.19.g042ed3e048af
Viresh Kumar
maintainerRe: [PATCH V2 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
Quoted 35 lines · Viresh Kumar
dev_pm_opp_set_bw()is getting removed anddev_pm_opp_set_opp()should be used instead. Migrate to the new API.We don't want the OPP core to manage the clk for this driver, migrate to
dev_pm_opp_of_add_table_noclk()to make suredev_pm_opp_set_opp()doesn't have any side effects.Signed-off-by Viresh Kumar <[email protected]>---
Dmitry,This is based over the patches sent here:
This should fix the problem you mentioned earlier. Will push this for linux-next unless you have any issues with it.
drivers/devfreq/tegra30-devfreq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/devfreq/tegra30-devfreq.c +1
-1@@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,return PTR_ERR (opp); } - ret =dev_pm_opp_set_bw (dev, opp); + ret =dev_pm_opp_set_opp (dev, opp);dev_pm_opp_put (opp);return ret;
drivers/devfreq/tegra30-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 852
Quoted 5 lines
drivers/devfreq/tegra30-devfreq.c +1
-1@@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)return err; } - err =dev_pm_opp_of_add_table (&pdev->dev); + err =dev_pm_opp_of_add_table_noclk (&pdev->dev);
Plus this, somehow was left uncommited in my tree :(
drivers/devfreq/tegra30-devfreq.c +1 -1
@@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)return err; } - err =dev_pm_opp_of_add_table_noclk (&pdev->dev); + err =dev_pm_opp_of_add_table_noclk (&pdev->dev,0 );if (err) {dev_err (&pdev->dev,"Failed to add OPP table: %d\n" , err); goto put_hw;
Signature · 1 line
viresh
Dmitry Osipenko
maintainerRe: [PATCH V2 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
27.01.2021 13:02, Viresh Kumar пишет:
Quoted 50 lines · Viresh Kumar
Quoted 35 lines
dev_pm_opp_set_bw()is getting removed anddev_pm_opp_set_opp()should be used instead. Migrate to the new API.We don't want the OPP core to manage the clk for this driver, migrate to
dev_pm_opp_of_add_table_noclk()to make suredev_pm_opp_set_opp()doesn't have any side effects.Signed-off-by Viresh Kumar <[email protected]>---
Dmitry,This is based over the patches sent here:
This should fix the problem you mentioned earlier. Will push this for linux-next unless you have any issues with it.
drivers/devfreq/tegra30-devfreq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/devfreq/tegra30-devfreq.c +1
-1@@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,return PTR_ERR (opp); } - ret =dev_pm_opp_set_bw (dev, opp); + ret =dev_pm_opp_set_opp (dev, opp);dev_pm_opp_put (opp);return ret;drivers/devfreq/tegra30-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 852
Quoted 5 lines
drivers/devfreq/tegra30-devfreq.c +1
-1@@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)return err; } - err =dev_pm_opp_of_add_table (&pdev->dev); + err =dev_pm_opp_of_add_table_noclk (&pdev->dev);Plus this, somehow was left uncommited in my tree :(
drivers/devfreq/tegra30-devfreq.c +0
-0
drivers/devfreq/tegra30-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 855
Quoted 8 lines
drivers/devfreq/tegra30-devfreq.c +1
-1@@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)return err; } - err =dev_pm_opp_of_add_table_noclk (&pdev->dev); + err =dev_pm_opp_of_add_table_noclk (&pdev->dev,0 );if (err) {dev_err (&pdev->dev,"Failed to add OPP table: %d\n" , err); goto put_hw;
Sadly this doesn't work because we missed that clk is assigned to opp_table when OPP table is allocated and not when it's added to device.
Hence we're now set back to the dev_pm_opp_clear_clk() variant.
What about to add a new OPP API which will allow OPP users to configure behaviour that user wants from OPP core in a generic way, something like this:
struct opp_config {
bool no_clk;
...
};devm_pm_opp_set_config(dev, struct opp_config);dev_pm_opp_set_config(dev, struct opp_config);dev_pm_opp_unset_config(dev);
Or maybe even rename it dev_pm_opp_allocate_table(dev, struct opp_config), which will allow users to directly allocate OPP table instead of relying on the implicit allocations. Then there won't be a need for drivers to use a dummy devm_pm_opp_set_clkname(dev, NULL) just to allocate the table usable for dev_pm_opp_set_rate().
Viresh Kumar
maintainerRe: [PATCH V2 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
Quoted 2 lines · Dmitry Osipenko
Sadly this doesn't work because we missed that clk is assigned to
opp_tablewhen OPP table is allocated and not when it's added to device.
Ahh, I missed that.
I have bumped up the other patchset to V2, that should work fine with this patch for tegra30 (this shouldn't require any update).
Everything is pushed in opp/linux-next, fetch and try it. Thanks.
Signature · 1 line
viresh
Re: [PATCH V2 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
In reply to Viresh KumarQuoted 36 lines · Viresh Kumar
dev_pm_opp_set_bw()is getting removed anddev_pm_opp_set_opp()should be used instead. Migrate to the new API.We don't want the OPP core to manage the clk for this driver, migrate to
dev_pm_opp_of_add_table_noclk()to make suredev_pm_opp_set_opp()doesn't have any side effects.Signed-off-by Viresh Kumar <[email protected]>---
Dmitry,This is based over the patches sent here:
This should fix the problem you mentioned earlier. Will push this for linux-next unless you have any issues with it.
drivers/devfreq/tegra30-devfreq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/devfreq/tegra30-devfreq.c +1
-1@@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,return PTR_ERR (opp); } - ret =dev_pm_opp_set_bw (dev, opp); + ret =dev_pm_opp_set_opp (dev, opp);dev_pm_opp_put (opp);return ret;
drivers/devfreq/tegra30-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 855
Quoted 8 lines
drivers/devfreq/tegra30-devfreq.c +1
-1@@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)return err; } - err =dev_pm_opp_of_add_table (&pdev->dev); + err =dev_pm_opp_of_add_table_noclk (&pdev->dev);if (err) {dev_err (&pdev->dev,"Failed to add OPP table: %d\n" , err); goto put_hw;
Signature · 3 lines
Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH V2 11/13] devfreq: tegra30: Migrate to dev_pm_opp_set_opp()
In reply to Viresh Kumar27.01.2021 12:10, Viresh Kumar пишет:
Quoted 36 lines
dev_pm_opp_set_bw()is getting removed anddev_pm_opp_set_opp()should be used instead. Migrate to the new API.We don't want the OPP core to manage the clk for this driver, migrate to
dev_pm_opp_of_add_table_noclk()to make suredev_pm_opp_set_opp()doesn't have any side effects.Signed-off-by Viresh Kumar <[email protected]>---
Dmitry,This is based over the patches sent here:
This should fix the problem you mentioned earlier. Will push this for linux-next unless you have any issues with it.
drivers/devfreq/tegra30-devfreq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/devfreq/tegra30-devfreq.c +1
-1@@ -647,7 +647,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,return PTR_ERR (opp); } - ret =dev_pm_opp_set_bw (dev, opp); + ret =dev_pm_opp_set_opp (dev, opp);dev_pm_opp_put (opp);return ret;
drivers/devfreq/tegra30-devfreq.c static int tegra_devfreq_probe(struct platform_device *pdev) · line 855
Quoted 8 lines
drivers/devfreq/tegra30-devfreq.c +1
-1@@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)return err; } - err =dev_pm_opp_of_add_table (&pdev->dev); + err =dev_pm_opp_of_add_table_noclk (&pdev->dev);if (err) {dev_err (&pdev->dev,"Failed to add OPP table: %d\n" , err); goto put_hw;
Viresh Kumar
maintainerPATCH 12/13 drm: msm: Migrate to dev_pm_opp_set_opp()
In reply to Viresh Kumardev_pm_opp_set_bw() is getting removed and dev_pm_opp_set_opp() should be used instead. Migrate to the new API.
--- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/gpu/drm/msm/adreno/a6xx_gmu.c +4 -4
@@ -134,7 +134,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)if (!gmu->legacy) {a6xx_hfi_set_freq (gmu, perf_index); -dev_pm_opp_set_bw (&gpu->pdev->dev, opp); +dev_pm_opp_set_opp (&gpu->pdev->dev, opp);pm_runtime_put (gmu->dev);return ; }
@@ -158,7 +158,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)if (ret)dev_err (gmu->dev,"GMU set GPU frequency error: %d\n" , ret); -dev_pm_opp_set_bw (&gpu->pdev->dev, opp); +dev_pm_opp_set_opp (&gpu->pdev->dev, opp);pm_runtime_put (gmu->dev); }
@@ -866,7 +866,7 @@ static void a6xx_gmu_set_initial_bw(struct msm_gpu *gpu, struct a6xx_gmu *gmu)if (IS_ERR_OR_NULL (gpu_opp))return ; -dev_pm_opp_set_bw (&gpu->pdev->dev, gpu_opp); +dev_pm_opp_set_opp (&gpu->pdev->dev, gpu_opp);dev_pm_opp_put (gpu_opp); }
@@ -1072,7 +1072,7 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)a6xx_gmu_shutdown (gmu);/* Remove the bus vote */ -dev_pm_opp_set_bw (&gpu->pdev->dev,NULL ); +dev_pm_opp_set_opp (&gpu->pdev->dev,NULL );/* * Make sure the GX domain is off before turning off the GMU (CX)
Signature · 7 lines
2.25.0.rc1.19.g042ed3e048af _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Viresh Kumar
maintainerPATCH 13/13 opp: Remove dev_pm_opp_set_bw()
In reply to Viresh KumarAll the users have migrated to dev_pm_opp_set_opp() now, get rid of the duplicate API, dev_pm_opp_set_bw(), which only performs a part of the new API.
While at it, remove the unnecessary parameter to _set_opp_bw().
--- drivers/opp/core.c | 41 +++++------------------------------------ include/linux/pm_opp.h | 6 ------ 2 files changed, 5 insertions(+), 42 deletions(-)
drivers/opp/core.c +5 -36
@@ -798,7 +798,7 @@ static int _generic_set_opp_regulator(struct opp_table *opp_table, }static int _set_opp_bw (const struct opp_table *opp_table, -struct dev_pm_opp *opp,struct device *dev,bool remove) +struct dev_pm_opp *opp,struct device *dev) {u32 avg, peak;int i, ret;
@@ -807,7 +807,7 @@ static int _set_opp_bw(const struct opp_table *opp_table,return 0 ;for (i =0 ; i < opp_table->path_count; i++) { -if (remove) { +if (!opp) { avg =0 ; peak =0 ; }else {
@@ -817,7 +817,7 @@ static int _set_opp_bw(const struct opp_table *opp_table, ret =icc_set_bw (opp_table->paths[i], avg, peak);if (ret) {dev_err (dev,"Failed to %s bandwidth[%d]: %d\n" , - remove ?"remove" :"set" , i, ret); + opp ?"set" :"remove" , i, ret);return ret; } }
@@ -911,37 +911,6 @@ static int _set_required_opps(struct device *dev,return ret; } -/** -* dev_pm_opp_set_bw() - sets bandwidth levels corresponding to an opp -* @dev: device for which we do this operation -* @opp: opp based on which the bandwidth levels are to be configured -* -* This configures the bandwidth to the levels specified by the OPP. However -* if the OPP specified is NULL the bandwidth levels are cleared out. -* -* Return: 0 on success or a negative error value. -*/ -int dev_pm_opp_set_bw (struct device *dev,struct dev_pm_opp *opp) -{ -struct opp_table *opp_table; -int ret; - - opp_table =_find_opp_table (dev); -if (IS_ERR (opp_table)) { -dev_err (dev,"%s: device opp table doesn't exist\n" , __func__); -return PTR_ERR (opp_table); - } - -if (opp) - ret =_set_opp_bw (opp_table, opp, dev, false); -else - ret =_set_opp_bw (opp_table,NULL , dev, true); - -dev_pm_opp_put_opp_table (opp_table); -return ret; -} -EXPORT_SYMBOL_GPL (dev_pm_opp_set_bw); -static void _find_current_opp (struct device *dev,struct opp_table *opp_table) {struct dev_pm_opp *opp =ERR_PTR (-ENODEV );
@@ -982,7 +951,7 @@ static int _disable_opp_table(struct device *dev, struct opp_table *opp_table)if (!_get_opp_count (opp_table))return 0 ; - ret =_set_opp_bw (opp_table,NULL , dev, true); + ret =_set_opp_bw (opp_table,NULL , dev);if (ret)return ret;
@@ -1050,7 +1019,7 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, }if (!ret) { - ret =_set_opp_bw (opp_table, opp, dev, false); + ret =_set_opp_bw (opp_table, opp, dev);if (!ret) { opp_table->enabled = true;dev_pm_opp_put (old_opp);
include/linux/pm_opp.h +0 -6
@@ -159,7 +159,6 @@ struct opp_table *devm_pm_opp_attach_genpd(struct device *dev, const char **nameint dev_pm_opp_xlate_performance_state (struct opp_table *src_table,struct opp_table *dst_table,unsigned int pstate);int dev_pm_opp_set_rate (struct device *dev,unsigned long target_freq);int dev_pm_opp_set_opp (struct device *dev,struct dev_pm_opp *opp); -int dev_pm_opp_set_bw (struct device *dev,struct dev_pm_opp *opp);int dev_pm_opp_set_sharing_cpus (struct device *cpu_dev,const struct cpumask *cpumask);int dev_pm_opp_get_sharing_cpus (struct device *cpu_dev,struct cpumask *cpumask);void dev_pm_opp_remove_table (struct device *dev);
@@ -382,11 +381,6 @@ static inline int dev_pm_opp_set_opp(struct device *dev, struct dev_pm_opp *opp)return -ENOTSUPP ; } -static inline int dev_pm_opp_set_bw (struct device *dev,struct dev_pm_opp *opp) -{ -return -EOPNOTSUPP ; -} -static inline int dev_pm_opp_set_sharing_cpus (struct device *cpu_dev,const struct cpumask *cpumask) {return -ENOTSUPP ;
Signature · 7 lines
2.25.0.rc1.19.g042ed3e048af _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel