* Re: [PATCH v2 3/5] OPP: Add support for parsing the interconnect bandwidth
From: Sibi Sankar @ 2019-06-27 6:27 UTC (permalink / raw)
To: Viresh Kumar, Georgi Djakov
Cc: vireshk, sboyd, nm, robh+dt, mark.rutland, rjw, jcrouse,
vincent.guittot, bjorn.andersson, amit.kucheria, seansw,
daidavid1, evgreen, linux-pm, devicetree, linux-kernel,
linux-arm-msm
In-Reply-To: <20190424055208.kermzymve2foguhl@vireshk-i7>
Hey Georgi,
In addition to Viresh's comments I found a few more while testing the
series on SDM845.
On 4/24/19 11:22 AM, Viresh Kumar wrote:
> On 23-04-19, 16:28, Georgi Djakov wrote:
>> The OPP bindings now support bandwidth values, so add support to parse it
>> from device tree and store it into the new dev_pm_opp_icc_bw struct, which
>> is part of the dev_pm_opp.
>>
>> Also add and export the dev_pm_opp_set_paths() and dev_pm_opp_put_paths()
>> helpers, to set (and release) an interconnect paths to a device. The
>> bandwidth of these paths will be updated when the OPPs are switched.
>>
>> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
>> ---
>> drivers/opp/core.c | 87 ++++++++++++++++++++++++++++++++++-
>> drivers/opp/of.c | 102 +++++++++++++++++++++++++++++++++++++++++
>> drivers/opp/opp.h | 9 ++++
>> include/linux/pm_opp.h | 14 ++++++
>> 4 files changed, 210 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 0420f7e8ad5b..97ee39ecdebd 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -19,6 +19,7 @@
>> #include <linux/slab.h>
>> #include <linux/device.h>
>> #include <linux/export.h>
>> +#include <linux/interconnect.h>
>
> Just include this once in opp.h and the other .c files won't need it.
>
>> #include <linux/pm_domain.h>
>> #include <linux/regulator/consumer.h>
>>
>> @@ -876,6 +877,8 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
>> ret);
>> }
>>
>> + _of_find_paths(opp_table, dev);
>> +
>> BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head);
>> INIT_LIST_HEAD(&opp_table->opp_list);
>> kref_init(&opp_table->kref);
>> @@ -1129,11 +1132,12 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_remove_all_dynamic);
>> struct dev_pm_opp *_opp_allocate(struct opp_table *table)
>> {
>> struct dev_pm_opp *opp;
>> - int count, supply_size;
>> + int count, supply_size, icc_size;
>>
>> /* Allocate space for at least one supply */
>> count = table->regulator_count > 0 ? table->regulator_count : 1;
>> supply_size = sizeof(*opp->supplies) * count;
>> + icc_size = sizeof(*opp->bandwidth) * table->path_count;
>>
>> /* allocate new OPP node and supplies structures */
>> opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
>
> You never updated this to include icc_size :(
>
>> @@ -1141,7 +1145,8 @@ struct dev_pm_opp *_opp_allocate(struct opp_table *table)
>> return NULL;
>>
>> /* Put the supplies at the end of the OPP structure as an empty array */
>> - opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
>> + opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp + 1);
>
> Keep the order as supplies and then bandwidth.
>
>> + opp->supplies = (struct dev_pm_opp_supply *)(opp + icc_size + 1);
opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies + 1);
>
> Did you check what address gets assigned here ? I think the pointer
> addition will screw things up for you.
>
>> INIT_LIST_HEAD(&opp->node);
>>
>> return opp;
>> @@ -1637,6 +1642,84 @@ void dev_pm_opp_put_clkname(struct opp_table *opp_table)
>> }
>> EXPORT_SYMBOL_GPL(dev_pm_opp_put_clkname);
>>
>> +/**
>> + * dev_pm_opp_set_paths() - Set interconnect path for a device
>> + * @dev: Device for which interconnect path is being set.
>> + *
>> + * This must be called before any OPPs are initialized for the device.
>> + */
>> +struct opp_table *dev_pm_opp_set_paths(struct device *dev)
>
> I got a bit confused. Why is this routine required exactly as
> _of_find_paths() would have already done something similar ?
>
>> +{
>> + struct opp_table *opp_table;
>> + int ret, i;
>> +
>> + opp_table = dev_pm_opp_get_opp_table(dev);
>> + if (!opp_table)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + /* This should be called before OPPs are initialized */
>> + if (WARN_ON(!list_empty(&opp_table->opp_list))) {
>> + ret = -EBUSY;
>> + goto err;
>> + }
>> +
>> + /* Another CPU that shares the OPP table has set the path */
>> + if (opp_table->paths)
>> + return opp_table;
>> + >> + opp_table->paths = kmalloc_array(opp_table->path_count,
of_find_paths might have failed so you would want to
re-calculate opp_table->path_count.
>> + sizeof(*opp_table->paths), GFP_KERNEL);
>> +
>> + /* Find interconnect path(s) for the device */
>> + for (i = 0; i < opp_table->path_count; i++) {
>> + opp_table->paths[i] = of_icc_get_by_index(dev, i);
>> + if (IS_ERR(opp_table->paths[i])) {
>> + ret = PTR_ERR(opp_table->paths[i]);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "%s: Couldn't find path%d: %d\n",
>> + __func__, i, ret);
we should clean up by call icc_put on the paths that succeeded
and free/set the opp_table->paths to NULL.
>> + goto err;
>> + }
>> + }
>> +
>> + return opp_table;
>> +
>> +err:
>> + dev_pm_opp_put_opp_table(opp_table);
>> +
>> + return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL_GPL(dev_pm_opp_set_paths);
>> +
>> +/**
>> + * dev_pm_opp_put_paths() - Release interconnect path resources
>> + * @opp_table: OPP table returned from dev_pm_opp_set_paths().
>> + */
>> +void dev_pm_opp_put_paths(struct opp_table *opp_table)
>> +{
>> + int i;
>> +
>> + if (!opp_table->paths) {
>> + pr_err("%s: Doesn't have paths set\n", __func__);
>> + return;
>> + }
>> +
>> + /* Make sure there are no concurrent readers while updating opp_table */
>> + WARN_ON(!list_empty(&opp_table->opp_list));
>> +
>> + for (i = opp_table->path_count - 1; i >= 0; i--)
>> + icc_put(opp_table->paths[i]);
>> +
>> + _free_set_opp_data(opp_table);
>> +
>> + kfree(opp_table->paths);
>> + opp_table->paths = NULL;
>> + opp_table->path_count = 0;
>> +
>> + dev_pm_opp_put_opp_table(opp_table);
>> +}
>> +EXPORT_SYMBOL_GPL(dev_pm_opp_put_paths);
>> +
>> /**
>> * dev_pm_opp_register_set_opp_helper() - Register custom set OPP helper
>> * @dev: Device for which the helper is getting registered.
>> diff --git a/drivers/opp/of.c b/drivers/opp/of.c
>> index c10c782d15aa..00af23280bc6 100644
>> --- a/drivers/opp/of.c
>> +++ b/drivers/opp/of.c
>> @@ -16,6 +16,7 @@
>> #include <linux/cpu.h>
>> #include <linux/errno.h>
>> #include <linux/device.h>
>> +#include <linux/interconnect.h>
>> #include <linux/of_device.h>
>> #include <linux/pm_domain.h>
>> #include <linux/slab.h>
>> @@ -363,6 +364,45 @@ static int _of_opp_alloc_required_opps(struct opp_table *opp_table,
>> return ret;
>> }
>>
>> +int _of_find_paths(struct opp_table *opp_table, struct device *dev)
>> +{
>> + struct device_node *np;
>> + int ret, i, count, num_paths;
>> +
>> + np = of_node_get(dev->of_node);
>> + if (np) {
>
> I would rather do:
>
> if (!np)
> return 0;
>
> That will kill unnecessary line breaks and indentation.
>
>> + count = of_count_phandle_with_args(np, "interconnects",
>> + "#interconnect-cells");
count will be an error code if interconnect property is missing
so we need to account for that as well.
>
> You can do of_node_put() right here as it isn't used afterwards.
>
>> + if (count % 2) {
>
> Shouldn't this be 4 instead of 2 ? Each path is represented as:
>
> <&noc MASTER_CPU &noc SLAVE_DDR>
>
> which has 4 fields.
>
>> + dev_err(dev, "%s: Invalid interconnects values\n",
>> + __func__);
>> + ret = -EINVAL;
>> + goto put_of_node;
>> + }
>> +
>> + num_paths = count / 2;
>> + opp_table->paths = kcalloc(num_paths, sizeof(*opp_table->paths),
>> + GFP_KERNEL);
>> + if (!opp_table->paths) {
>> + ret = -ENOMEM;
>> + goto put_of_node;
>> + }
>> +
>> + for (i = 0; i < num_paths; i++)
>> + opp_table->paths[i] = of_icc_get_by_index(dev, i);
we should clean up by call icc_put on the paths that succeeded
and free/set the opp_table->paths to NULL.
>> +
>> + opp_table->path_count = num_paths;
>> + of_node_put(np);
>> + }
>> +
>> + return 0;
>> +
>> +put_of_node:
>> + of_node_put(np);
>> +
>> + return ret;
>> +}
>> +
>> static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
>> struct device_node *np)
>> {
>> @@ -539,6 +579,64 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
>> return ret;
>> }
>>
>> +static int opp_parse_icc_bw(struct dev_pm_opp *opp, struct device *dev,
>> + struct opp_table *opp_table)
>> +{
>> + struct property *prop = NULL;
>> + u32 *bandwidth;
>> + char name[] = "bandwidth-MBps";
>> + int count, i, j, ret;
>> +
>> + /* Search for "bandwidth-MBps" */
>> + prop = of_find_property(opp->np, name, NULL);
>> +
>> + /* Missing property is not a problem */
>> + if (!prop) {
>> + dev_dbg(dev, "%s: Missing %s property\n", __func__, name);
>> + return 0;
>> + }
>> +
>> + if (!prop->value) {
>> + dev_dbg(dev, "%s: Missing %s value\n", __func__, name);
>> + return -ENODATA;
>> + }
>> +
>> + /*
>> + * Bandwidth consists of average and peak values like:
>> + * bandwidth-MBps = <avg-MBps peak-MBps>
>> + */
>> + count = prop->length / sizeof(u32);
>> + if (count % 2) {
>> + dev_err(dev, "%s: Invalid %s values\n", __func__, name);
>> + return -EINVAL;
>> + }
>> +
>> + if (opp_table->path_count != count / 2) {
>> + dev_err(dev, "%s Mismatch between values and paths (%d %d)\n",
>> + __func__, opp_table->path_count, count / 2);
>> + return -EINVAL;
>> + }
>> +
>> + bandwidth = kmalloc_array(count, sizeof(*bandwidth), GFP_KERNEL);
>> + if (!bandwidth)
>> + return -ENOMEM;
>> +
>> + ret = of_property_read_u32_array(opp->np, name, bandwidth, count);
>> + if (ret) {
>> + dev_err(dev, "%s: Error parsing %s: %d\n", __func__, name, ret);
>> + goto free_bandwidth;
>> + }
>> + for (i = 0, j = 0; i < count; i++) {
>> + opp->bandwidth[i].avg = MBps_to_icc(bandwidth[j++]);
>> + opp->bandwidth[i].peak = MBps_to_icc(bandwidth[j++]);
>> + }
>> +
>> +free_bandwidth:
>> + kfree(bandwidth);
>> +
>> + return ret;
>> +}
>> +
>> /**
>> * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT
>> * entries
>> @@ -635,6 +733,10 @@ 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_parse_icc_bw(new_opp, dev, opp_table);
>> + if (ret)
>> + goto free_opp;
>> +
>> ret = _opp_add(dev, new_opp, opp_table, rate_not_available);
>> if (ret) {
>> /* Don't return error for duplicate OPPs */
>> diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
>> index 569b3525aa67..70a537f2dbd3 100644
>> --- a/drivers/opp/opp.h
>> +++ b/drivers/opp/opp.h
>> @@ -24,6 +24,7 @@
>>
>> struct clk;
>> struct regulator;
>> +struct icc_path;
>>
>> /* Lock to allow exclusive modification to the device and opp lists */
>> extern struct mutex opp_table_lock;
>> @@ -62,6 +63,7 @@ extern struct list_head opp_tables;
>> * @rate: Frequency in hertz
>> * @level: Performance level
>> * @supplies: Power supplies voltage/current values
>> + * @bandwidth: Interconnect bandwidth values
>> * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
>> * frequency from any other OPP's frequency.
>> * @required_opps: List of OPPs that are required by this OPP.
>> @@ -84,6 +86,7 @@ struct dev_pm_opp {
>> unsigned int level;
>>
>> struct dev_pm_opp_supply *supplies;
>> + struct dev_pm_opp_icc_bw *bandwidth;
>>
>> unsigned long clock_latency_ns;
>>
>> @@ -150,6 +153,8 @@ enum opp_table_access {
>> * @regulator_count: Number of power supply regulators. Its value can be -1
>> * (uninitialized), 0 (no opp-microvolt property) or > 0 (has opp-microvolt
>> * property).
>> + * @paths: Interconnect path handles
>> + * @path_count: Number of interconnect paths
>> * @genpd_performance_state: Device's power domain support performance state.
>> * @is_genpd: Marks if the OPP table belongs to a genpd.
>> * @set_opp: Platform specific set_opp callback
>> @@ -194,6 +199,8 @@ struct opp_table {
>> struct clk *clk;
>> struct regulator **regulators;
>> int regulator_count;
>> + struct icc_path **paths;
>> + unsigned int path_count;
>> bool genpd_performance_state;
>> bool is_genpd;
>>
>> @@ -228,12 +235,14 @@ void _of_clear_opp_table(struct opp_table *opp_table);
>> struct opp_table *_managed_opp(struct device *dev, int index);
>> void _of_opp_free_required_opps(struct opp_table *opp_table,
>> struct dev_pm_opp *opp);
>> +int _of_find_paths(struct opp_table *opp_table, struct device *dev);
>> #else
>> static inline void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) {}
>> static inline void _of_clear_opp_table(struct opp_table *opp_table) {}
>> static inline struct opp_table *_managed_opp(struct device *dev, int index) { return NULL; }
>> static inline void _of_opp_free_required_opps(struct opp_table *opp_table,
>> struct dev_pm_opp *opp) {}
>> +static inline int _of_find_paths(struct opp_table *opp_table, struct device *dev) { return 0; }
>> #endif
>>
>> #ifdef CONFIG_DEBUG_FS
>> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
>> index 24c757a32a7b..dabee09a92b8 100644
>> --- a/include/linux/pm_opp.h
>> +++ b/include/linux/pm_opp.h
>> @@ -43,6 +43,18 @@ struct dev_pm_opp_supply {
>> unsigned long u_amp;
>> };
>>
>> +/**
>> + * struct dev_pm_opp_icc_bw - Interconnect bandwidth values
>> + * @avg: Average bandwidth corresponding to this OPP (in icc units)
>> + * @peak: Peak bandwidth corresponding to this OPP (in icc units)
>> + *
>> + * This structure stores the bandwidth values for a single interconnect path.
>> + */
>> +struct dev_pm_opp_icc_bw {
>> + u32 avg;
>> + u32 peak;
>> +};
>> +
>> /**
>> * struct dev_pm_opp_info - OPP freq/voltage/current values
>> * @rate: Target clk rate in hz
>> @@ -127,6 +139,8 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev, const char * con
>> void dev_pm_opp_put_regulators(struct opp_table *opp_table);
>> struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char * name);
>> void dev_pm_opp_put_clkname(struct opp_table *opp_table);
>> +struct opp_table *dev_pm_opp_set_paths(struct device *dev);
>> +void dev_pm_opp_put_paths(struct opp_table *opp_table);
>> struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev, int (*set_opp)(struct dev_pm_set_opp_data *data));
>> void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table);
>> struct opp_table *dev_pm_opp_set_genpd_virt_dev(struct device *dev, struct device *virt_dev, int index);
>
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v2 2/5] interconnect: Add of_icc_get_by_index() helper function
From: Sibi Sankar @ 2019-06-27 5:56 UTC (permalink / raw)
To: Georgi Djakov, vireshk, sboyd, nm, robh+dt, mark.rutland, rjw
Cc: jcrouse, vincent.guittot, bjorn.andersson, amit.kucheria, seansw,
daidavid1, evgreen, linux-pm, devicetree, linux-kernel,
linux-arm-msm, linux-kernel-owner
In-Reply-To: <e6469e3b-3653-d20b-b27d-242547a777df@codeaurora.org>
Hey Georgi,
I heard there is a follow up discussion
planned to finalize on the which approach
to follow. If we do end up with your series,
I found some fixes that you might want to
use when you re-post.
On 2019-05-07 17:29, Sibi Sankar wrote:
> Hey Georgi,
>
> On 4/23/19 6:58 PM, Georgi Djakov wrote:
>> This is the same as the traditional of_icc_get() function, but the
>> difference is that it takes index as an argument, instead of name.
>>
>> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
>> ---
>> drivers/interconnect/core.c | 45
>> ++++++++++++++++++++++++++++--------
>> include/linux/interconnect.h | 6 +++++
>> 2 files changed, 41 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
>> index 871eb4bc4efc..a7c3c262c974 100644
>> --- a/drivers/interconnect/core.c
>> +++ b/drivers/interconnect/core.c
>> @@ -295,9 +295,9 @@ static struct icc_node
>> *of_icc_get_from_provider(struct of_phandle_args *spec)
>> }
>> /**
>> - * of_icc_get() - get a path handle from a DT node based on name
>> + * of_icc_get_by_index() - get a path handle from a DT node based on
>> index
>> * @dev: device pointer for the consumer device
>> - * @name: interconnect path name
>> + * @idx: interconnect path index
>> *
>> * This function will search for a path between two endpoints and
>> return an
>> * icc_path handle on success. Use icc_put() to release constraints
>> when they
>> @@ -309,13 +309,12 @@ static struct icc_node
>> *of_icc_get_from_provider(struct of_phandle_args *spec)
>> * Return: icc_path pointer on success or ERR_PTR() on error. NULL
>> is returned
>> * when the API is disabled or the "interconnects" DT property is
>> missing.
>> */
>> -struct icc_path *of_icc_get(struct device *dev, const char *name)
>> +struct icc_path *of_icc_get_by_index(struct device *dev, int idx)
>> {
>> struct icc_path *path = ERR_PTR(-EPROBE_DEFER);
>> struct icc_node *src_node, *dst_node;
>> struct device_node *np = NULL;
>> struct of_phandle_args src_args, dst_args;
>> - int idx = 0;
>> int ret;
>> if (!dev || !dev->of_node)
>> @@ -335,12 +334,6 @@ struct icc_path *of_icc_get(struct device *dev,
>> const char *name)
>> * lets support only global ids and extend this in the future if
>> needed
>> * without breaking DT compatibility.
>> */
>> - if (name) {
>> - idx = of_property_match_string(np, "interconnect-names", name);
>> - if (idx < 0)
>> - return ERR_PTR(idx);
>> - }
>> -
>> ret = of_parse_phandle_with_args(np, "interconnects",
>> "#interconnect-cells", idx * 2,
>> &src_args);
>> @@ -383,6 +376,38 @@ struct icc_path *of_icc_get(struct device *dev,
>> const char *name)
>> return path;
>> }
>> +
>> +/**
>> + * of_icc_get() - get a path handle from a DT node based on name
>> + * @dev: device pointer for the consumer device
>> + * @name: interconnect path name
>> + *
>> + * This function will search for a path between two endpoints and
>> return an
>> + * icc_path handle on success. Use icc_put() to release constraints
>> when they
>> + * are not needed anymore.
>> + * If the interconnect API is disabled, NULL is returned and the
>> consumer
>> + * drivers will still build. Drivers are free to handle this
>> specifically,
>> + * but they don't have to.
>> + *
>> + * Return: icc_path pointer on success or ERR_PTR() on error. NULL is
>> returned
>> + * when the API is disabled or the "interconnects" DT property is
>> missing.
>> + */
please change the description since it does not
return NULL when the property is missing.
>> +struct icc_path *of_icc_get(struct device *dev, const char *name)
>> +{
>> + int idx = 0;
>> +
>> + if (!dev || !dev->of_node)
>> + return ERR_PTR(-ENODEV);
>> +
>> + if (name) {
>> + idx = of_property_match_string(dev->of_node,
>> + "interconnect-names", name);
>> + if (idx < 0)
>> + return ERR_PTR(idx);
>> + }
>> +
>> + return of_icc_get_by_index(dev, idx);
>> +}
>> EXPORT_SYMBOL_GPL(of_icc_get);
>> /**
>> diff --git a/include/linux/interconnect.h
>> b/include/linux/interconnect.h
>> index dc25864755ba..0e430b3b6519 100644
>> --- a/include/linux/interconnect.h
>> +++ b/include/linux/interconnect.h
>> @@ -28,6 +28,7 @@ struct device;
>> struct icc_path *icc_get(struct device *dev, const int src_id,
>> const int dst_id);
>> struct icc_path *of_icc_get(struct device *dev, const char *name);
>> +struct icc_path *of_icc_get_by_index(struct device *dev, int idx);
>> void icc_put(struct icc_path *path);
>> int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw);
>> @@ -45,6 +46,11 @@ static inline struct icc_path *of_icc_get(struct
>> device *dev,
>> return NULL;
>> }
>> +struct icc_path *of_icc_get_by_index(struct device *dev, int idx)
>
> This should be static inline instead
>
>> +{
>> + return NULL;
>> +}
>> +
>> static inline void icc_put(struct icc_path *path)
>> {
>> }
>>
--
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH V2 2/5] cpufreq: Replace few CPUFREQ_CONST_LOOPS checks with has_target()
From: Viresh Kumar @ 2019-06-27 5:00 UTC (permalink / raw)
To: Rafael Wysocki; +Cc: linux-pm, Vincent Guittot, linux-kernel
In-Reply-To: <88da7cfabad5e19a361fe2843e5ef547d50fd221.1560999838.git.viresh.kumar@linaro.org>
On 20-06-19, 08:35, Viresh Kumar wrote:
> > CPUFREQ_CONST_LOOPS was introduced in a very old commit from pre-2.6
> > kernel release commit 6a4a93f9c0d5 ("[CPUFREQ] Fix 'out of sync'
> > issue").
> >
> > Probably the initial idea was to just avoid these checks for set_policy
> > type drivers and then things got changed over the years. And it is very
> > unclear why these checks are there at all.
> >
> > Replace the CPUFREQ_CONST_LOOPS check with has_target(), which makes
> > more sense now.
> >
> > cpufreq_notify_transition() is only called for has_target() type driver
> > and not for set_policy type, and the check is simply redundant. Remove
> > it as well.
> >
> > Also remove () around freq comparison statement as they aren't required
> > and checkpatch also warns for them.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> > drivers/cpufreq/cpufreq.c | 13 +++++--------
> > 1 file changed, 5 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 54befd775bd6..41ac701e324f 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -359,12 +359,10 @@ static void cpufreq_notify_transition(struct cpufreq_policy *policy,
> > * which is not equal to what the cpufreq core thinks is
> > * "old frequency".
> > */
> > - if (!(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) {
> > - if (policy->cur && (policy->cur != freqs->old)) {
> > - pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n",
> > - freqs->old, policy->cur);
> > - freqs->old = policy->cur;
> > - }
> > + if (policy->cur && policy->cur != freqs->old) {
> > + pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n",
> > + freqs->old, policy->cur);
> > + freqs->old = policy->cur;
> > }
> >
> > srcu_notifier_call_chain(&cpufreq_transition_notifier_list,
> > @@ -1618,8 +1616,7 @@ static unsigned int __cpufreq_get(struct cpufreq_policy *policy)
> > if (policy->fast_switch_enabled)
> > return ret_freq;
> >
> > - if (ret_freq && policy->cur &&
> > - !(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) {
> > + if (has_target() && ret_freq && policy->cur) {
> > /* verify no discrepancy between actual and
> > saved value exists */
> > if (unlikely(ret_freq != policy->cur)) {
@Rafael: Here are your comments from the IRC exchange we had
yesterday:
> <rafael>:
>
> so the problem is that, because of the CPUFREQ_CONST_LOOPS check in
> __cpufreq_get(), it almost never does the cpufreq_out_of_sync() thing
> now. Because many drivers set CPUFREQ_CONST_LOOPS most of the time,
> some of them even unconditionally. This patch changes the code that
> runs very rarely into code that runs relatively often.
Right, we will do the frequency verification on has_target() platforms
with CPUFREQ_CONST_LOOPS set after this patch. But why is it the wrong
thing to do ?
What we do here is that we verify that the cached value of current
frequency is same as the real frequency the hardware is running at. It
makes sense to not do this check for setpolicy type drivers as the
cpufreq core isn't always aware of what the driver will end up doing
with the frequency and so no verification.
But for has_target() type drivers, cpufreq core caches the value with
it and it should check it to make sure everything is fine. I don't see
a correlation with CPUFREQ_CONST_LOOPS flag here, that's it. Either we
do this verification or we don't, but there is no reason (as per my
understanding) of skipping it using this flag.
So if you look at the commit I pointed in the history git [1], it does
two things:
- It adds the verification code (which is quite similar today as
well).
- And it sets the CPUFREQ_CONST_LOOPS flag only for setpolicy drivers,
rightly so.
The problem happened when we started to use CPUFREQ_CONST_LOOPS for
constant loops-per-jiffy thing as well and many has_target() drivers
started using the same flag and unknowingly skipped the verification
of frequency.
So, I think the current code is doing the wrong thing by skipping the
verification using CPUFREQ_CONST_LOOPS flag.
--
viresh
[1] https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=6a4a93f9c0d51b5f4ac1bd3efab53e43584330dd
^ permalink raw reply
* Re: [PATCH v4 4/5] Documentation: devicetree: add PPMU events description
From: Chanwoo Choi @ 2019-06-27 1:11 UTC (permalink / raw)
To: Lukasz Luba, Krzysztof Kozlowski
Cc: cwchoi00, devicetree, linux-kernel, linux-pm,
linux-samsung-soc@vger.kernel.org, linux-arm-kernel,
Bartłomiej Żołnierkiewicz, robh+dt, mark.rutland,
kyungmin.park, Marek Szyprowski, s.nawrocki, myungjoo.ham, kgene,
willy.mh.wolff.ml
In-Reply-To: <776f58c2-a05c-8fa8-c7f5-458dc17926f6@partner.samsung.com>
Hi Lukasz,
On 19. 6. 26. 오후 11:17, Lukasz Luba wrote:
> Hi Krzysztof,
>
> On 6/26/19 4:03 PM, Krzysztof Kozlowski wrote:
>> On Wed, 26 Jun 2019 at 15:58, Lukasz Luba <l.luba@partner.samsung.com> wrote:
>>>
>>> Hi Chanwoo,
>>>
>>> On 6/26/19 10:23 AM, Chanwoo Choi wrote:
>>>> Hi Lukasz,
>>>>
>>>> 2019년 6월 5일 (수) 18:14, Lukasz Luba <l.luba@partner.samsung.com
>>>> <mailto:l.luba@partner.samsung.com>>님이 작성:
>>>>
>>>> Extend the documenation by events description with new 'event-data-type'
>>>> field. Add example how the event might be defined in DT.
>>>>
>>>> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com
>>>> <mailto:l.luba@partner.samsung.com>>
>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com
>>>> <mailto:cw00.choi@samsung.com>>
>>>> ---
>>>> .../bindings/devfreq/event/exynos-ppmu.txt | 26 +++++++++++++++++--
>>>> 1 file changed, 24 insertions(+), 2 deletions(-)
>>>>
>>>>
>>>>
>>>> Acked-by: Chanwoo Choi <cw00.choi@samsung.com
>>>
>>> Thank you for the ACKs for this a 2/5 patch.
>>> Do you think the v4 could be merged now?
>>
>> I think you have all necessary acks. I can take the DTS patch (5/5)
>> although probably for next merge window as I just sent one.
> There was one patch 3/5
> https://protect2.fireeye.com/url?k=82dd0d0cbe2abd04.82dc8643-d13ecd7e5f989b8d&u=https://lkml.org/lkml/2019/6/5/215
> which was waiting ACK or I missed the email somehow.
When I was in vacation, your patches are removed on my email account
because of the email expiration. So, I replied with my Ack through
gmail account on mobile phone. But, there are some problem. My reply
didn't arrive the mailing list.
I have no any way to reply about this at company. After leaving one's
office, I'll reply with Ack again at home.
>
> Regards,
> Lukasz
>
>>
>> Best regards,
>> Krzysztof
>>
>>
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* Re: [PATCH] PM / wakeup: show wakeup sources stats in sysfs
From: Greg KH @ 2019-06-27 0:04 UTC (permalink / raw)
To: Tri Vo
Cc: Rafael J. Wysocki, Viresh Kumar, Rafael J. Wysocki,
Hridya Valsaraju, Sandeep Patil, LKML, Linux PM,
Cc: Android Kernel
In-Reply-To: <CANA+-vD+qBqENZrk_7KZzedbzGPMzHniHTE4sY93gnkzzBif6A@mail.gmail.com>
On Wed, Jun 26, 2019 at 03:48:58PM -0700, Tri Vo wrote:
> On Tue, Jun 25, 2019 at 6:33 PM Tri Vo <trong@android.com> wrote:
> >
> > On Tue, Jun 25, 2019 at 6:12 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, Jun 25, 2019 at 05:54:49PM -0700, Tri Vo wrote:
> > > > Embedding a struct kobject into struct wakeup_source changes lifetime
> > > > requirements on the latter. To that end, change deallocation of struct
> > > > wakeup_source using kfree to kobject_put().
> > >
> > > Ick, are you sure you need a new kobject here? Why wouldn't a named
> > > attribute group work instead? That should keep this patch much smaller
> > > and simpler.
> >
> > Yeah, named attribute groups might be a much cleaner way to do this.
> > Let me investigate.
>
> Say, we read /sys/power/wakeup_sources/foo/active_count.
What is "foo" here? You didn't include Documentation of the sysfs
files so it was pretty impossible to say what exactly your heirachy was
going to be in order to determine this :)
> This
> attribute's show function needs to find wakeup_source struct of "foo".
> I'm not sure how to do that without embedding a kobject inside of
> wakeup_source.
Again, without knowing what "foo" is, I can't really answer this.
Surely "foo" is not a flat namespace of all 'struct device' in the
kernel, right?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] PM / wakeup: show wakeup sources stats in sysfs
From: Tri Vo @ 2019-06-26 22:48 UTC (permalink / raw)
To: Greg KH
Cc: Rafael J. Wysocki, Viresh Kumar, Rafael J. Wysocki,
Hridya Valsaraju, Sandeep Patil, LKML, Linux PM,
Cc: Android Kernel
In-Reply-To: <CANA+-vBoabFTD=fMz+0d5Sbe9rPwnxcuxJxaMCT3KAwXYHSD7w@mail.gmail.com>
On Tue, Jun 25, 2019 at 6:33 PM Tri Vo <trong@android.com> wrote:
>
> On Tue, Jun 25, 2019 at 6:12 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Jun 25, 2019 at 05:54:49PM -0700, Tri Vo wrote:
> > > Embedding a struct kobject into struct wakeup_source changes lifetime
> > > requirements on the latter. To that end, change deallocation of struct
> > > wakeup_source using kfree to kobject_put().
> >
> > Ick, are you sure you need a new kobject here? Why wouldn't a named
> > attribute group work instead? That should keep this patch much smaller
> > and simpler.
>
> Yeah, named attribute groups might be a much cleaner way to do this.
> Let me investigate.
Say, we read /sys/power/wakeup_sources/foo/active_count. This
attribute's show function needs to find wakeup_source struct of "foo".
I'm not sure how to do that without embedding a kobject inside of
wakeup_source.
^ permalink raw reply
* Re: [PATCH] PM / wakeup: show wakeup sources stats in sysfs
From: Tri Vo @ 2019-06-26 22:26 UTC (permalink / raw)
To: Greg KH
Cc: Rafael J. Wysocki, Viresh Kumar, Rafael J. Wysocki,
Hridya Valsaraju, Sandeep Patil, LKML, Linux PM,
Cc: Android Kernel
In-Reply-To: <20190626014633.GA22610@kroah.com>
On Tue, Jun 25, 2019 at 6:48 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jun 25, 2019 at 06:33:08PM -0700, Tri Vo wrote:
> > On Tue, Jun 25, 2019 at 6:12 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > +static ssize_t wakeup_source_count_show(struct wakeup_source *ws,
> > > > + struct wakeup_source_attribute *attr,
> > > > + char *buf)
> > > > +{
> > > > + unsigned long flags;
> > > > + unsigned long var;
> > > > +
> > > > + spin_lock_irqsave(&ws->lock, flags);
> > > > + if (strcmp(attr->attr.name, "active_count") == 0)
> > > > + var = ws->active_count;
> > > > + else if (strcmp(attr->attr.name, "event_count") == 0)
> > > > + var = ws->event_count;
> > > > + else if (strcmp(attr->attr.name, "wakeup_count") == 0)
> > > > + var = ws->wakeup_count;
> > > > + else
> > > > + var = ws->expire_count;
> > > > + spin_unlock_irqrestore(&ws->lock, flags);
> > > > +
> > > > + return sprintf(buf, "%lu\n", var);
> > > > +}
> > >
> > > Why is this lock always needed to be grabbed? You are just reading a
> > > value, who cares if it changes inbetween reading it and returning the
> > > buffer string as it can change at that point in time anyway?
> >
> > Right, we don't care if the value changes in between us reading and
> > printing it. However, IIUC not grabbing this lock results in a data
> > race, which is undefined behavior.
>
> A data race where? Writing to the value? How can that happen? All you
> are doing is incrementing this variable elsewhere, what is the worst
> that can happen?
Ok, I'll remove the locks.
^ permalink raw reply
* Re: [PATCH] PCI: PM: Avoid skipping bus-level PM on platforms without ACPI
From: Rafael J. Wysocki @ 2019-06-26 21:52 UTC (permalink / raw)
To: Mika Westerberg
Cc: Rafael J. Wysocki, Linux PCI, Linux PM, Jon Hunter, Bjorn Helgaas,
LKML, Kai-Heng Feng
In-Reply-To: <20190626125605.GT2640@lahna.fi.intel.com>
On Wed, Jun 26, 2019 at 2:56 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> On Wed, Jun 26, 2019 at 12:20:23AM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > There are platforms that do not call pm_set_suspend_via_firmware(),
> > so pm_suspend_via_firmware() returns 'false' on them, but the power
> > states of PCI devices (PCIe ports in particular) are changed as a
> > result of powering down core platform components during system-wide
> > suspend. Thus the pm_suspend_via_firmware() checks in
> > pci_pm_suspend_noirq() and pci_pm_resume_noirq() introduced by
> > commit 3e26c5feed2a ("PCI: PM: Skip devices in D0 for suspend-to-
> > idle") are not sufficient to determine that devices left in D0
> > during suspend will remain in D0 during resume and so the bus-level
> > power management can be skipped for them.
> >
> > For this reason, introduce a new global suspend flag,
> > PM_SUSPEND_FLAG_NO_PLATFORM, set it for suspend-to-idle only
> > and replace the pm_suspend_via_firmware() checks mentioned above
> > with checks against this flag.
> >
> > Fixes: 3e26c5feed2a ("PCI: PM: Skip devices in D0 for suspend-to-idle")
> > Reported-by: Jon Hunter <jonathanh@nvidia.com>
> > Tested-by: Jon Hunter <jonathanh@nvidia.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> I tested this patch on top of your (and mine) previous patches touching
> the ACPI/PCI PM and did not see any issues over several suspend-to-idle
> cycles with and without TBT device connected.
>
> Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Thanks!
^ permalink raw reply
* Re: cpupower: update German translation
From: shuah @ 2019-06-26 18:11 UTC (permalink / raw)
To: Dominik Brodowski, Benjamin Weis; +Cc: trenn, linux-pm, shuah
In-Reply-To: <20190624140931.GA21950@light.dominikbrodowski.net>
On 6/24/19 8:09 AM, Dominik Brodowski wrote:
> On Mon, Jun 24, 2019 at 03:31:50PM +0200, Benjamin Weis wrote:
>> Update the German translation of cpupower, and change the encoding
>> to UTF-8.
>>
>> Signed-off-by: Benjamin Weis <benjamin.weis@gmx.com>
>
> Acked-by: Dominik Brodowski <linux@dominikbrodowski.net>
>
Thanks Dominik!
Hi Benjamin,
Thanks for the patch. This patch appears to be corrupted. It is also
missing [PATCH] tag. Please refer to patch submission guidelines and
send v2.
Documentation/process/submitting-patches.rst
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v1 0/3] Add required-opps support to devfreq passive gov
From: Saravana Kannan @ 2019-06-26 18:10 UTC (permalink / raw)
To: Viresh Kumar
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Android Kernel Team, Linux PM, LKML
In-Reply-To: <20190626063240.kgdiy7xsz4mahrdr@vireshk-i7>
On Tue, Jun 25, 2019 at 11:32 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 24-06-19, 22:29, Saravana Kannan wrote:
> > No, the CPUs will be the "parent" and the cache will be the "child".
> > CPU is a special case when it comes to the actual software (not DT) as
> > we'll need the devfreq governor to look at all the CPUfreq policies to
> > decide the cache frequency (max of all their requirements).
> >
> > I think "master" and "slave" would have been a better term as the
> > master device determines its frequency using whatever means and the
> > "slave" device just "follows" the master device.
>
> Okay, so to confirm again this is what we will have:
>
> - CPUs are called masters and Caches are slaves.
>
> - The devfreq governor we are talking about takes care of changing
> frequency of caches (slaves) and chooses a target frequency for
> caches based on what the masters are running at.
>
> - The CPUs OPP nodes will have required-opps property and will be
> pointing to the caches OPP nodes. The CPUs may already be using
> required-opps node for PM domain performance state thing.
>
>
> Now the problem is "required-opp" means something really *required*
> and it is not optional.
But we could interpret it as "required" for different things.
> Like a specific voltage level before we can
> switch to a particular frequency.
Required for stability.
> And this is how I have though of it
> until now. And this shouldn't be handled asynchronously, i.e. required
> OPPs must be set while configuring OPP of a device.
The users of clocks are expected to set up the voltage correctly
before changing the frequency, the drivers are expected to power up
the device before trying to access its registers, etc. So I don't
think there is one correct way. Also OPP sets the pstate only if
dev_pm_opp_set_genpd_virt_dev() has been called to set them up. So
this is not a mandatory principle in the kernel that a framework
providing an API should have all the dependencies. So I don't think
we'll be violating some golden rule.
> So, when a CPU changes frequency, we must change the performance state
> of PM domain and change frequency/bw of the cache synchronously.
I mean, it's going to be changed when we get the CPUfreq transition
notifiers. From a correctness point of view, setting it inside the OPP
framework is not any better than doing it when we get the notifiers.
> And
> in such a case "required-opps" can be a very good fit for this use
> case.
Glad you agree :)
> But with what you are trying to do it is no longer required-opp but
> good-to-have-opp. And that worries me.
I see this as "required for good performance". So I don't see it as
redefining required-opps. If someone wants good performance/power
balance they follow the "required-opps". Technically even the PM
pstates are required for good power. Otherwise, the system could leave
the voltage at max and stuff would still work.
Also, the slave device might need to get input from multiple master
devices and aggregate the request before setting the slave device
frequency. So I don't think OPP framework would be the right place to
deal with those things. For example, L3 might (will) have different
mappings for big vs little cores. So that needs to be aggregated and
set properly by the slave device driver. Also, GPU might have a
mapping for L3 too. In which case the L3 slave driver needs to take
input from even more masters before it decides its frequency. But most
importantly, we still need the ability to change governors for L3.
Again these are just examples with L3 and it can get more complicated
based on the situation.
Most importantly, instead of always going by mapping, one might decide
to scale the L3 based on some other governor (that looks at some HW
counter). Or just set it to performance governor for a use case for
which performance is more important. All of this comes for free with
devfreq and if we always set it from OPP framework we don't give this
required control to userspace.
I think going through devfreq is the right approach for this. And we
can always rewrite the software if we find problems in the future. But
as it stands today, this will help cases like exynos without the need
for a lot of changes. Hope I've convinced you.
-Saravana
^ permalink raw reply
* Re: [PATCH v4 2/3] nvme: add thermal zone devices
From: Akinobu Mita @ 2019-06-26 15:52 UTC (permalink / raw)
To: Zhang Rui
Cc: linux-nvme, linux-pm, Eduardo Valentin, Daniel Lezcano,
Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Minwoo Im, Kenneth Heitke, Chaitanya Kulkarni
In-Reply-To: <1561474998.19713.13.camel@intel.com>
2019年6月26日(水) 0:03 Zhang Rui <rui.zhang@intel.com>:
>
> On 五, 2019-06-14 at 00:20 +0900, Akinobu Mita wrote:
> > The NVMe controller reports up to nine temperature values in the
> > SMART /
> > Health log page (the composite temperature and temperature sensor 1
> > through
> > temperature sensor 8).
> >
> > This provides these temperatures via thermal zone devices.
> >
> > Once the controller is identified, the thermal zone devices are
> > created for
> > all implemented temperature sensors including the composite
> > temperature.
> >
> > /sys/class/thermal/thermal_zone[0-*]:
> > |---type: 'nvme<instance>-temp<sensor>'
> > |---temp: Temperature
> > |---trip_point_0_temp: Over temperature threshold
> >
> > The thermal_zone[0-*] contains a 'device' symlink to the
> > corresponding nvme
> > device.
> >
> > On the other hand, the following symlinks to the thermal zone devices
> > are
> > created in the nvme device sysfs directory.
> >
> > - temp0: Composite temperature
> > - temp1: Temperature sensor 1
> > ...
> > - temp8: Temperature sensor 8
> >
> > In addition to the standard thermal zone device, this also adds
> > support for
> > registering the DT thermal zone device.
> >
> I don't see standard thermal zone device and DT thermal zone device are
> registered at the same time very often, especially if they represent
> the same sensor.
Good point.
It is pointless to register both standard and DT thermal zone devices for
the same sensor. We should register either one. (i.e. firstly try to
register DT thermal zone. If no thermal zones found for the sensor in
device tree, then try to register standard one)
^ permalink raw reply
* Re: [PATCH v4 4/5] Documentation: devicetree: add PPMU events description
From: Lukasz Luba @ 2019-06-26 14:17 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: cwchoi00, devicetree, linux-kernel, linux-pm,
linux-samsung-soc@vger.kernel.org, linux-arm-kernel,
Bartłomiej Żołnierkiewicz, robh+dt, mark.rutland,
Chanwoo Choi, kyungmin.park, Marek Szyprowski, s.nawrocki,
myungjoo.ham, kgene, willy.mh.wolff.ml
In-Reply-To: <CAJKOXPc6304D=HNQnrvhBH6qKxhkf=VQ2Gg6Q2FMP2hYOTYSDQ@mail.gmail.com>
Hi Krzysztof,
On 6/26/19 4:03 PM, Krzysztof Kozlowski wrote:
> On Wed, 26 Jun 2019 at 15:58, Lukasz Luba <l.luba@partner.samsung.com> wrote:
>>
>> Hi Chanwoo,
>>
>> On 6/26/19 10:23 AM, Chanwoo Choi wrote:
>>> Hi Lukasz,
>>>
>>> 2019년 6월 5일 (수) 18:14, Lukasz Luba <l.luba@partner.samsung.com
>>> <mailto:l.luba@partner.samsung.com>>님이 작성:
>>>
>>> Extend the documenation by events description with new 'event-data-type'
>>> field. Add example how the event might be defined in DT.
>>>
>>> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com
>>> <mailto:l.luba@partner.samsung.com>>
>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com
>>> <mailto:cw00.choi@samsung.com>>
>>> ---
>>> .../bindings/devfreq/event/exynos-ppmu.txt | 26 +++++++++++++++++--
>>> 1 file changed, 24 insertions(+), 2 deletions(-)
>>>
>>>
>>>
>>> Acked-by: Chanwoo Choi <cw00.choi@samsung.com
>>
>> Thank you for the ACKs for this a 2/5 patch.
>> Do you think the v4 could be merged now?
>
> I think you have all necessary acks. I can take the DTS patch (5/5)
> although probably for next merge window as I just sent one.
There was one patch 3/5
https://lkml.org/lkml/2019/6/5/215
which was waiting ACK or I missed the email somehow.
Regards,
Lukasz
>
> Best regards,
> Krzysztof
>
>
^ permalink raw reply
* Re: [PATCH v4 4/5] Documentation: devicetree: add PPMU events description
From: Krzysztof Kozlowski @ 2019-06-26 14:03 UTC (permalink / raw)
To: Lukasz Luba
Cc: cwchoi00, devicetree, linux-kernel, linux-pm,
linux-samsung-soc@vger.kernel.org, linux-arm-kernel,
Bartłomiej Żołnierkiewicz, robh+dt, mark.rutland,
Chanwoo Choi, kyungmin.park, Marek Szyprowski, s.nawrocki,
myungjoo.ham, kgene, willy.mh.wolff.ml
In-Reply-To: <7498059d-95f7-e154-cf49-bcbc8ee6fdb9@partner.samsung.com>
On Wed, 26 Jun 2019 at 15:58, Lukasz Luba <l.luba@partner.samsung.com> wrote:
>
> Hi Chanwoo,
>
> On 6/26/19 10:23 AM, Chanwoo Choi wrote:
> > Hi Lukasz,
> >
> > 2019년 6월 5일 (수) 18:14, Lukasz Luba <l.luba@partner.samsung.com
> > <mailto:l.luba@partner.samsung.com>>님이 작성:
> >
> > Extend the documenation by events description with new 'event-data-type'
> > field. Add example how the event might be defined in DT.
> >
> > Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com
> > <mailto:l.luba@partner.samsung.com>>
> > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com
> > <mailto:cw00.choi@samsung.com>>
> > ---
> > .../bindings/devfreq/event/exynos-ppmu.txt | 26 +++++++++++++++++--
> > 1 file changed, 24 insertions(+), 2 deletions(-)
> >
> >
> >
> > Acked-by: Chanwoo Choi <cw00.choi@samsung.com
>
> Thank you for the ACKs for this a 2/5 patch.
> Do you think the v4 could be merged now?
I think you have all necessary acks. I can take the DTS patch (5/5)
although probably for next merge window as I just sent one.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 4/5] Documentation: devicetree: add PPMU events description
From: Lukasz Luba @ 2019-06-26 13:58 UTC (permalink / raw)
To: cwchoi00
Cc: devicetree, linux-kernel, linux-pm, linux-samsung-soc,
linux-arm-kernel, b.zolnierkie, krzk, robh+dt, mark.rutland,
cw00.choi, kyungmin.park, m.szyprowski, s.nawrocki, myungjoo.ham,
kgene, willy.mh.wolff.ml
In-Reply-To: <CAGTfZH2kTNWtx=Jp1UJaLN50Qxbq+Q9ThV4vhQ240QbOy1TRMQ@mail.gmail.com>
Hi Chanwoo,
On 6/26/19 10:23 AM, Chanwoo Choi wrote:
> Hi Lukasz,
>
> 2019년 6월 5일 (수) 18:14, Lukasz Luba <l.luba@partner.samsung.com
> <mailto:l.luba@partner.samsung.com>>님이 작성:
>
> Extend the documenation by events description with new 'event-data-type'
> field. Add example how the event might be defined in DT.
>
> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com
> <mailto:l.luba@partner.samsung.com>>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com
> <mailto:cw00.choi@samsung.com>>
> ---
> .../bindings/devfreq/event/exynos-ppmu.txt | 26 +++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
>
>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com
Thank you for the ACKs for this a 2/5 patch.
Do you think the v4 could be merged now?
Regards,
Lukasz
^ permalink raw reply
* Re: [PATCH v4 2/2] dt-bindings: mfd: max8998: Add charger subnode binding
From: Lee Jones @ 2019-06-26 13:06 UTC (permalink / raw)
To: Paweł Chmiel
Cc: sre, robh+dt, mark.rutland, linux-kernel, linux-pm, devicetree,
linux-samsung-soc
In-Reply-To: <20190621115602.17559-3-pawel.mikolaj.chmiel@gmail.com>
On Fri, 21 Jun 2019, Paweł Chmiel wrote:
> This patch adds devicetree bindings documentation for
> battery charging controller as the subnode of MAX8998 PMIC.
It makes sense to place this in:
Documentation/devicetree/bindings/power/supply/
And link to it from this file using the following syntax:
See: ../power/supply/<file>.txt
> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> ---
> Changes from v3:
> - Property prefix should be maxim, not max8998
> - Describe what End of Charge in percent means
>
> Changes from v2:
> - Make charge-restart-level-microvolt optional.
> - Make charge-timeout-hours optional.
>
> Changes from v1:
> - Removed unneeded Fixes tag
> - Correct description of all charger values
> - Added missing property unit
> ---
> .../devicetree/bindings/mfd/max8998.txt | 26 +++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt
> index 5f2f07c09c90..368f787d6079 100644
> --- a/Documentation/devicetree/bindings/mfd/max8998.txt
> +++ b/Documentation/devicetree/bindings/mfd/max8998.txt
> @@ -48,6 +48,25 @@ Additional properties required if max8998,pmic-buck2-dvs-gpio is defined:
> - max8998,pmic-buck2-dvs-voltage: An array of 2 voltage values in microvolts
> for buck2 regulator that can be selected using dvs gpio.
>
> +Charger: Configuration for battery charging controller should be added
> +inside a child node named 'charger'.
> + Required properties:
> + - maxim,end-of-charge-percentage: End of Charge in percent.
> + When the charge current in constant-voltage phase drops below
> + end-of-charge-percentage of it's start value, charging is terminated.
> + If value equals 0, leave it unchanged. Otherwise it should be value
> + from 10 to 45 by 5 step.
> +
> + Optional properties:
> + - maxim,charge-restart-threshold: Charge restart threshold in millivolts.
> + If property is not present, this will be disabled.
> + Valid values are: 0, 100, 150, 200. If the value equals 0, leave it
> + unchanged.
> +
> + - maxim,charge-timeout: Charge timeout in hours. If property is not
> + present, this will be disabled. Valid values are: 0, 5, 6, 7.
> + If the value equals 0, leave it unchanged.
> +
> Regulators: All the regulators of MAX8998 to be instantiated shall be
> listed in a child node named 'regulators'. Each regulator is represented
> by a child node of the 'regulators' node.
> @@ -97,6 +116,13 @@ Example:
> max8998,pmic-buck2-dvs-gpio = <&gpx0 0 3 0 0>; /* SET3 */
> max8998,pmic-buck2-dvs-voltage = <1350000>, <1300000>;
>
> + /* Charger configuration */
> + charger {
> + maxim,end-of-charge-percentage = <20>;
> + maxim,charge-restart-threshold = <100>;
> + maxim,charge-timeout = <7>;
> + };
> +
> /* Regulators to instantiate */
> regulators {
> ldo2_reg: LDO2 {
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] PCI: PM: Avoid skipping bus-level PM on platforms without ACPI
From: Mika Westerberg @ 2019-06-26 12:56 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PCI, Linux PM, Jon Hunter, Bjorn Helgaas, LKML,
Kai-Heng Feng
In-Reply-To: <14605632.7Eqku7tdey@kreacher>
On Wed, Jun 26, 2019 at 12:20:23AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> There are platforms that do not call pm_set_suspend_via_firmware(),
> so pm_suspend_via_firmware() returns 'false' on them, but the power
> states of PCI devices (PCIe ports in particular) are changed as a
> result of powering down core platform components during system-wide
> suspend. Thus the pm_suspend_via_firmware() checks in
> pci_pm_suspend_noirq() and pci_pm_resume_noirq() introduced by
> commit 3e26c5feed2a ("PCI: PM: Skip devices in D0 for suspend-to-
> idle") are not sufficient to determine that devices left in D0
> during suspend will remain in D0 during resume and so the bus-level
> power management can be skipped for them.
>
> For this reason, introduce a new global suspend flag,
> PM_SUSPEND_FLAG_NO_PLATFORM, set it for suspend-to-idle only
> and replace the pm_suspend_via_firmware() checks mentioned above
> with checks against this flag.
>
> Fixes: 3e26c5feed2a ("PCI: PM: Skip devices in D0 for suspend-to-idle")
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Tested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
I tested this patch on top of your (and mine) previous patches touching
the ACPI/PCI PM and did not see any issues over several suspend-to-idle
cycles with and without TBT device connected.
Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
^ permalink raw reply
* Re: [PATCH V3 2/3] thermal/drivers/cpu_cooling: Unregister with the policy
From: Daniel Lezcano @ 2019-06-26 12:52 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Viresh Kumar, Rafael J. Wysocki, Eduardo Valentin,
Linux Kernel Mailing List, Sudeep Holla, Amit Daniel Kachhap,
Javi Merino, Zhang Rui, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team, Keerthy,
open list:CPU FREQUENCY DRIVERS - ARM BIG LITTLE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:TI BANDGAP AND THERMAL DRIVER
In-Reply-To: <CAJZ5v0h7=BqnQqvULnQr3MuQsS2qwSn7RCZbMo-V+cUi+kbvSg@mail.gmail.com>
On 26/06/2019 13:28, Rafael J. Wysocki wrote:
> On Wed, Jun 26, 2019 at 12:19 PM Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>>
>> On 26/06/2019 11:06, Rafael J. Wysocki wrote:
>>> On Wed, Jun 26, 2019 at 8:37 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>>>
>>>> On 26-06-19, 08:02, Daniel Lezcano wrote:
>>>>> On 26/06/2019 04:58, Viresh Kumar wrote:
>>>>>> On 25-06-19, 13:32, Daniel Lezcano wrote:
>>>>>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>>>>>>> index aee024e42618..f07454249fbc 100644
>>>>>>> --- a/drivers/cpufreq/cpufreq.c
>>>>>>> +++ b/drivers/cpufreq/cpufreq.c
>>>>>>> @@ -1379,8 +1379,8 @@ static int cpufreq_online(unsigned int cpu)
>>>>>>> cpufreq_driver->ready(policy);
>>>>>>>
>>>>>>> if (cpufreq_thermal_control_enabled(cpufreq_driver))
>>>>>>> - policy->cdev = of_cpufreq_cooling_register(policy);
>>>>>>> -
>>>>>>> + of_cpufreq_cooling_register(policy);
>>>>>>> +
>>>>>>
>>>>>> We don't need any error checking here anymore ?
>>>>>
>>>>> There was no error checking initially. This comment and the others below
>>>>> are for an additional patch IMO, not a change in this one.
>>>>
>>>> right, but ...
>>>>
>>>>>>> -void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>>>>>>> +void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
>>>>>>> {
>>>>>>> struct cpufreq_cooling_device *cpufreq_cdev;
>>>>>>> bool last;
>>>>>>>
>>>>>>> - if (!cdev)
>>>>>>> - return;
>>>>
>>>> we used to return without any errors from here. Now we will have
>>>> problems if regsitering fails for some reason.
>>>
>>> Specifically, the last cpufreq_cdev in the list will be unregistered
>>> AFAICS, and without removing it from the list for that matter, which
>>> isn't what the caller wants.
>>
>> Indeed,
>>
>> What about the resulting code above:
>>
>> void __cpufreq_cooling_unregister(struct cpufreq_cooling_device
>> *cpufreq_cdev, int last)
>> {
>> /* Unregister the notifier for the last cpufreq cooling device */
>> if (last)
>> cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
>> CPUFREQ_POLICY_NOTIFIER);
>>
>
> Doesn't the notifier need to be unregistered under cooling_list_lock ?
I don't think so because the element is no longer in the list and we
don't touch the list anymore. Do you see another possible race?
>> thermal_cooling_device_unregister(cpufreq_cdev->cdev);
>> ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
>> kfree(cpufreq_cdev->idle_time);
>> kfree(cpufreq_cdev);
>> }
>>
>> /**
>>
>> * cpufreq_cooling_unregister - function to remove cpufreq cooling
>> device.
>> * @cdev: thermal cooling device pointer.
>>
>> *
>>
>> * This interface function unregisters the "thermal-cpufreq-%x" cooling
>> device.
>> */
>> void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
>> {
>> struct cpufreq_cooling_device *cpufreq_cdev;
>> bool last;
>>
>> mutex_lock(&cooling_list_lock);
>> list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
>> if (cpufreq_cdev->policy == policy) {
>> list_del(&cpufreq_cdev->node);
>> last = list_empty(&cpufreq_cdev_list);
>> break;
>> }
>> }
>> mutex_unlock(&cooling_list_lock);
>>
>> if (cpufreq_cdev->policy == policy)
>> __cpufreq_cooling_unregister(cpufreq_cdev, last);
>> }
>> EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);
>>
>>
>>
>>
>> --
>> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog
>>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH V3 2/3] thermal/drivers/cpu_cooling: Unregister with the policy
From: Rafael J. Wysocki @ 2019-06-26 11:28 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Rafael J. Wysocki, Viresh Kumar, Rafael J. Wysocki,
Eduardo Valentin, Linux Kernel Mailing List, Sudeep Holla,
Amit Daniel Kachhap, Javi Merino, Zhang Rui, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Keerthy,
open list:CPU FREQUENCY DRIVERS - ARM BIG LITTLE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:TI BANDGAP AND THERMAL DRIVER
In-Reply-To: <8a9b7bd0-9b21-1ce1-6176-cffff4b8d739@linaro.org>
On Wed, Jun 26, 2019 at 12:19 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 26/06/2019 11:06, Rafael J. Wysocki wrote:
> > On Wed, Jun 26, 2019 at 8:37 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >>
> >> On 26-06-19, 08:02, Daniel Lezcano wrote:
> >>> On 26/06/2019 04:58, Viresh Kumar wrote:
> >>>> On 25-06-19, 13:32, Daniel Lezcano wrote:
> >>>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> >>>>> index aee024e42618..f07454249fbc 100644
> >>>>> --- a/drivers/cpufreq/cpufreq.c
> >>>>> +++ b/drivers/cpufreq/cpufreq.c
> >>>>> @@ -1379,8 +1379,8 @@ static int cpufreq_online(unsigned int cpu)
> >>>>> cpufreq_driver->ready(policy);
> >>>>>
> >>>>> if (cpufreq_thermal_control_enabled(cpufreq_driver))
> >>>>> - policy->cdev = of_cpufreq_cooling_register(policy);
> >>>>> -
> >>>>> + of_cpufreq_cooling_register(policy);
> >>>>> +
> >>>>
> >>>> We don't need any error checking here anymore ?
> >>>
> >>> There was no error checking initially. This comment and the others below
> >>> are for an additional patch IMO, not a change in this one.
> >>
> >> right, but ...
> >>
> >>>>> -void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
> >>>>> +void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
> >>>>> {
> >>>>> struct cpufreq_cooling_device *cpufreq_cdev;
> >>>>> bool last;
> >>>>>
> >>>>> - if (!cdev)
> >>>>> - return;
> >>
> >> we used to return without any errors from here. Now we will have
> >> problems if regsitering fails for some reason.
> >
> > Specifically, the last cpufreq_cdev in the list will be unregistered
> > AFAICS, and without removing it from the list for that matter, which
> > isn't what the caller wants.
>
> Indeed,
>
> What about the resulting code above:
>
> void __cpufreq_cooling_unregister(struct cpufreq_cooling_device
> *cpufreq_cdev, int last)
> {
> /* Unregister the notifier for the last cpufreq cooling device */
> if (last)
> cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
> CPUFREQ_POLICY_NOTIFIER);
>
Doesn't the notifier need to be unregistered under cooling_list_lock ?
> thermal_cooling_device_unregister(cpufreq_cdev->cdev);
> ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
> kfree(cpufreq_cdev->idle_time);
> kfree(cpufreq_cdev);
> }
>
> /**
>
> * cpufreq_cooling_unregister - function to remove cpufreq cooling
> device.
> * @cdev: thermal cooling device pointer.
>
> *
>
> * This interface function unregisters the "thermal-cpufreq-%x" cooling
> device.
> */
> void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
> {
> struct cpufreq_cooling_device *cpufreq_cdev;
> bool last;
>
> mutex_lock(&cooling_list_lock);
> list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
> if (cpufreq_cdev->policy == policy) {
> list_del(&cpufreq_cdev->node);
> last = list_empty(&cpufreq_cdev_list);
> break;
> }
> }
> mutex_unlock(&cooling_list_lock);
>
> if (cpufreq_cdev->policy == policy)
> __cpufreq_cooling_unregister(cpufreq_cdev, last);
> }
> EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);
>
>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
^ permalink raw reply
* Re: [PATCH v2] PCI: PM: Skip devices in D0 for suspend-to-idle
From: Mika Westerberg @ 2019-06-26 10:58 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Jon Hunter, Linux PCI, Bjorn Helgaas, Linux PM, Linux ACPI, LKML,
Keith Busch, Kai-Heng Feng, linux-tegra
In-Reply-To: <CAJZ5v0gU9OedmZBNDGefG3GjS7FHRmgQ67eOcr2vXRrAg3zZbg@mail.gmail.com>
On Tue, Jun 25, 2019 at 06:23:46PM +0200, Rafael J. Wysocki wrote:
> > So I wonder if the patch below makes any difference?
>
> Mika, can you please test this one in combination with the other
> changes we've been working on?
Sure, I'll give it a try shortly.
^ permalink raw reply
* Re: [PATCH V3 2/3] thermal/drivers/cpu_cooling: Unregister with the policy
From: Daniel Lezcano @ 2019-06-26 10:19 UTC (permalink / raw)
To: Rafael J. Wysocki, Viresh Kumar
Cc: Rafael J. Wysocki, Eduardo Valentin, Linux Kernel Mailing List,
Sudeep Holla, Amit Daniel Kachhap, Javi Merino, Zhang Rui,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Keerthy,
open list:CPU FREQUENCY DRIVERS - ARM BIG LITTLE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:TI BANDGAP AND THERMAL DRIVER
In-Reply-To: <CAJZ5v0jFXmJ3ikEPQUp-cLv3+ZSnp1kP8CxdkZVofV1BS3+UwQ@mail.gmail.com>
On 26/06/2019 11:06, Rafael J. Wysocki wrote:
> On Wed, Jun 26, 2019 at 8:37 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>
>> On 26-06-19, 08:02, Daniel Lezcano wrote:
>>> On 26/06/2019 04:58, Viresh Kumar wrote:
>>>> On 25-06-19, 13:32, Daniel Lezcano wrote:
>>>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>>>>> index aee024e42618..f07454249fbc 100644
>>>>> --- a/drivers/cpufreq/cpufreq.c
>>>>> +++ b/drivers/cpufreq/cpufreq.c
>>>>> @@ -1379,8 +1379,8 @@ static int cpufreq_online(unsigned int cpu)
>>>>> cpufreq_driver->ready(policy);
>>>>>
>>>>> if (cpufreq_thermal_control_enabled(cpufreq_driver))
>>>>> - policy->cdev = of_cpufreq_cooling_register(policy);
>>>>> -
>>>>> + of_cpufreq_cooling_register(policy);
>>>>> +
>>>>
>>>> We don't need any error checking here anymore ?
>>>
>>> There was no error checking initially. This comment and the others below
>>> are for an additional patch IMO, not a change in this one.
>>
>> right, but ...
>>
>>>>> -void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>>>>> +void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
>>>>> {
>>>>> struct cpufreq_cooling_device *cpufreq_cdev;
>>>>> bool last;
>>>>>
>>>>> - if (!cdev)
>>>>> - return;
>>
>> we used to return without any errors from here. Now we will have
>> problems if regsitering fails for some reason.
>
> Specifically, the last cpufreq_cdev in the list will be unregistered
> AFAICS, and without removing it from the list for that matter, which
> isn't what the caller wants.
Indeed,
What about the resulting code above:
void __cpufreq_cooling_unregister(struct cpufreq_cooling_device
*cpufreq_cdev, int last)
{
/* Unregister the notifier for the last cpufreq cooling device */
if (last)
cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
CPUFREQ_POLICY_NOTIFIER);
thermal_cooling_device_unregister(cpufreq_cdev->cdev);
ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
kfree(cpufreq_cdev->idle_time);
kfree(cpufreq_cdev);
}
/**
* cpufreq_cooling_unregister - function to remove cpufreq cooling
device.
* @cdev: thermal cooling device pointer.
*
* This interface function unregisters the "thermal-cpufreq-%x" cooling
device.
*/
void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
{
struct cpufreq_cooling_device *cpufreq_cdev;
bool last;
mutex_lock(&cooling_list_lock);
list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
if (cpufreq_cdev->policy == policy) {
list_del(&cpufreq_cdev->node);
last = list_empty(&cpufreq_cdev_list);
break;
}
}
mutex_unlock(&cooling_list_lock);
if (cpufreq_cdev->policy == policy)
__cpufreq_cooling_unregister(cpufreq_cdev, last);
}
EXPORT_SYMBOL_GPL(cpufreq_cooling_unregister);
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH v2 1/1] cpuidle-powernv : forced wakeup for stop states
From: Abhishek @ 2019-06-26 9:09 UTC (permalink / raw)
To: Nicholas Piggin, linux-kernel, linux-pm, linuxppc-dev
Cc: daniel.lezcano, dja, ego, mpe, rjw
In-Reply-To: <1560938644.5ukemauqsy.astroid@bobo.none>
Hi Nick,
On 06/19/2019 03:39 PM, Nicholas Piggin wrote:
> Abhishek's on June 19, 2019 7:08 pm:
>> Hi Nick,
>>
>> Thanks for the review. Some replies below.
>>
>> On 06/19/2019 09:53 AM, Nicholas Piggin wrote:
>>> Abhishek Goel's on June 17, 2019 7:56 pm:
>>>> Currently, the cpuidle governors determine what idle state a idling CPU
>>>> should enter into based on heuristics that depend on the idle history on
>>>> that CPU. Given that no predictive heuristic is perfect, there are cases
>>>> where the governor predicts a shallow idle state, hoping that the CPU will
>>>> be busy soon. However, if no new workload is scheduled on that CPU in the
>>>> near future, the CPU may end up in the shallow state.
>>>>
>>>> This is problematic, when the predicted state in the aforementioned
>>>> scenario is a shallow stop state on a tickless system. As we might get
>>>> stuck into shallow states for hours, in absence of ticks or interrupts.
>>>>
>>>> To address this, We forcefully wakeup the cpu by setting the
>>>> decrementer. The decrementer is set to a value that corresponds with the
>>>> residency of the next available state. Thus firing up a timer that will
>>>> forcefully wakeup the cpu. Few such iterations will essentially train the
>>>> governor to select a deeper state for that cpu, as the timer here
>>>> corresponds to the next available cpuidle state residency. Thus, cpu will
>>>> eventually end up in the deepest possible state.
>>>>
>>>> Signed-off-by: Abhishek Goel <huntbag@linux.vnet.ibm.com>
>>>> ---
>>>>
>>>> Auto-promotion
>>>> v1 : started as auto promotion logic for cpuidle states in generic
>>>> driver
>>>> v2 : Removed timeout_needed and rebased the code to upstream kernel
>>>> Forced-wakeup
>>>> v1 : New patch with name of forced wakeup started
>>>> v2 : Extending the forced wakeup logic for all states. Setting the
>>>> decrementer instead of queuing up a hrtimer to implement the logic.
>>>>
>>>> drivers/cpuidle/cpuidle-powernv.c | 38 +++++++++++++++++++++++++++++++
>>>> 1 file changed, 38 insertions(+)
>>>>
>>>> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
>>>> index 84b1ebe212b3..bc9ca18ae7e3 100644
>>>> --- a/drivers/cpuidle/cpuidle-powernv.c
>>>> +++ b/drivers/cpuidle/cpuidle-powernv.c
>>>> @@ -46,6 +46,26 @@ static struct stop_psscr_table stop_psscr_table[CPUIDLE_STATE_MAX] __read_mostly
>>>> static u64 default_snooze_timeout __read_mostly;
>>>> static bool snooze_timeout_en __read_mostly;
>>>>
>>>> +static u64 forced_wakeup_timeout(struct cpuidle_device *dev,
>>>> + struct cpuidle_driver *drv,
>>>> + int index)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = index + 1; i < drv->state_count; i++) {
>>>> + struct cpuidle_state *s = &drv->states[i];
>>>> + struct cpuidle_state_usage *su = &dev->states_usage[i];
>>>> +
>>>> + if (s->disabled || su->disable)
>>>> + continue;
>>>> +
>>>> + return (s->target_residency + 2 * s->exit_latency) *
>>>> + tb_ticks_per_usec;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>> It would be nice to not have this kind of loop iteration in the
>>> idle fast path. Can we add a flag or something to the idle state?
>> Currently, we do not have any callback notification or some feedback that
>> notifies the driver everytime some state is enabled/disabled. So we have
>> to parse everytime to get the next enabled state.
> Ahh, that's why you're doing that.
>
>> Are you suggesting to
>> add something like next_enabled_state in cpuidle state structure itself
>> which will be updated when a state is enabled or disabled?
> Hmm, I guess it normally should not iterate over more than one state
> unless some idle states are disabled.
>
> What would have been nice is each state just have its own timeout
> field with ticks already calculated, if that could be updated when
> a state is enabled or disabled. How hard is that to add to the
> cpuidle core?
I have implemented a prototype which does what you have asked for. Added
a disable_callback which will update timeout whenever a state is
enabled or
disabled. But It would mean adding some code to cpuidle.h and
cpuidle/sysfs.c.
If that is not an issue, should I go ahead and post it?
>>>> +
>>>> static u64 get_snooze_timeout(struct cpuidle_device *dev,
>>>> struct cpuidle_driver *drv,
>>>> int index)
>>>> @@ -144,8 +164,26 @@ static int stop_loop(struct cpuidle_device *dev,
>>>> struct cpuidle_driver *drv,
>>>> int index)
>>>> {
>>>> + u64 dec_expiry_tb, dec, timeout_tb, forced_wakeup;
>>>> +
>>>> + dec = mfspr(SPRN_DEC);
>>>> + timeout_tb = forced_wakeup_timeout(dev, drv, index);
>>>> + forced_wakeup = 0;
>>>> +
>>>> + if (timeout_tb && timeout_tb < dec) {
>>>> + forced_wakeup = 1;
>>>> + dec_expiry_tb = mftb() + dec;
>>>> + }
>>> The compiler probably can't optimise away the SPR manipulations so try
>>> to avoid them if possible.
>> Are you suggesting something like set_dec_before_idle?(in line with
>> what you have suggested to do after idle, reset_dec_after_idle)
> I should have been clear, I meant don't mfspr(SPRN_DEC) until you
> have tested timeout_tb.
>
>>>> +
>>>> + if (forced_wakeup)
>>>> + mtspr(SPRN_DEC, timeout_tb);
>>> This should just be put in the above 'if'.
>> Fair point.
>>>> +
>>>> power9_idle_type(stop_psscr_table[index].val,
>>>> stop_psscr_table[index].mask);
>>>> +
>>>> + if (forced_wakeup)
>>>> + mtspr(SPRN_DEC, dec_expiry_tb - mftb());
>>> This will sometimes go negative and result in another timer interrupt.
>>>
>>> It also breaks irq work (which can be set here by machine check I
>>> believe.
>>>
>>> May need to implement some timer code to do this for you.
>>>
>>> static void reset_dec_after_idle(void)
>>> {
>>> u64 now;
>>> u64 *next_tb;
>>>
>>> if (test_irq_work_pending())
>>> return;
>>> now = mftb;
>>> next_tb = this_cpu_ptr(&decrementers_next_tb);
>>>
>>> if (now >= *next_tb)
>>> return;
>>> set_dec(*next_tb - now);
>>> if (test_irq_work_pending())
>>> set_dec(1);
>>> }
>>>
>>> Something vaguely like that. See timer_interrupt().
>> Ah, Okay. Will go through timer_interrupt().
> Thanks,
> Nick
Thanks,
Abhishek
^ permalink raw reply
* Re: [PATCH V3 2/3] thermal/drivers/cpu_cooling: Unregister with the policy
From: Rafael J. Wysocki @ 2019-06-26 9:06 UTC (permalink / raw)
To: Viresh Kumar
Cc: Daniel Lezcano, Rafael J. Wysocki, Eduardo Valentin,
Linux Kernel Mailing List, Sudeep Holla, Amit Daniel Kachhap,
Javi Merino, Zhang Rui, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team, Keerthy,
open list:CPU FREQUENCY DRIVERS - ARM BIG LITTLE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:TI BANDGAP AND THERMAL DRIVER
In-Reply-To: <20190626063716.cechnzsb75q5lclr@vireshk-i7>
On Wed, Jun 26, 2019 at 8:37 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 26-06-19, 08:02, Daniel Lezcano wrote:
> > On 26/06/2019 04:58, Viresh Kumar wrote:
> > > On 25-06-19, 13:32, Daniel Lezcano wrote:
> > >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > >> index aee024e42618..f07454249fbc 100644
> > >> --- a/drivers/cpufreq/cpufreq.c
> > >> +++ b/drivers/cpufreq/cpufreq.c
> > >> @@ -1379,8 +1379,8 @@ static int cpufreq_online(unsigned int cpu)
> > >> cpufreq_driver->ready(policy);
> > >>
> > >> if (cpufreq_thermal_control_enabled(cpufreq_driver))
> > >> - policy->cdev = of_cpufreq_cooling_register(policy);
> > >> -
> > >> + of_cpufreq_cooling_register(policy);
> > >> +
> > >
> > > We don't need any error checking here anymore ?
> >
> > There was no error checking initially. This comment and the others below
> > are for an additional patch IMO, not a change in this one.
>
> right, but ...
>
> > >> -void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
> > >> +void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
> > >> {
> > >> struct cpufreq_cooling_device *cpufreq_cdev;
> > >> bool last;
> > >>
> > >> - if (!cdev)
> > >> - return;
>
> we used to return without any errors from here. Now we will have
> problems if regsitering fails for some reason.
Specifically, the last cpufreq_cdev in the list will be unregistered
AFAICS, and without removing it from the list for that matter, which
isn't what the caller wants.
^ permalink raw reply
* Re: [patch 1/5] drivers/cpuidle: add cpuidle-haltpoll driver
From: Rafael J. Wysocki @ 2019-06-26 8:40 UTC (permalink / raw)
To: Marcelo Tosatti
Cc: kvm-devel, Paolo Bonzini, Radim Krcmar, Andrea Arcangeli,
Rafael J. Wysocki, Peter Zijlstra, Wanpeng Li,
Konrad Rzeszutek Wilk, Raslan KarimAllah, Boris Ostrovsky,
Ankur Arora, Christian Borntraeger, Linux PM
In-Reply-To: <20190613225022.932697232@redhat.com>
On Fri, Jun 14, 2019 at 12:55 AM Marcelo Tosatti <mtosatti@redhat.com> wrote:
>
> Add a cpuidle driver that calls the architecture default_idle routine.
>
> To be used in conjunction with the haltpoll governor.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> ---
> arch/x86/kernel/process.c | 2 -
> drivers/cpuidle/Kconfig | 9 +++++
> drivers/cpuidle/Makefile | 1
> drivers/cpuidle/cpuidle-haltpoll.c | 65 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 76 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.git/arch/x86/kernel/process.c
> ===================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/process.c 2019-06-13 16:19:27.877064340 -0400
> +++ linux-2.6.git/arch/x86/kernel/process.c 2019-06-13 16:19:48.795544892 -0400
> @@ -580,7 +580,7 @@
> safe_halt();
> trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
> }
> -#ifdef CONFIG_APM_MODULE
> +#if defined(CONFIG_APM_MODULE) || defined(CONFIG_HALTPOLL_CPUIDLE_MODULE)
> EXPORT_SYMBOL(default_idle);
> #endif
>
> Index: linux-2.6.git/drivers/cpuidle/Kconfig
> ===================================================================
> --- linux-2.6.git.orig/drivers/cpuidle/Kconfig 2019-06-13 16:19:27.878064316 -0400
> +++ linux-2.6.git/drivers/cpuidle/Kconfig 2019-06-13 18:41:40.599912671 -0400
> @@ -51,6 +51,15 @@
> source "drivers/cpuidle/Kconfig.powerpc"
> endmenu
>
> +config HALTPOLL_CPUIDLE
> + tristate "Halt poll cpuidle driver"
> + depends on X86
> + default y
> + help
> + This option enables halt poll cpuidle driver, which allows to poll
> + before halting in the guest (more efficient than polling in the
> + host via halt_poll_ns for some scenarios).
> +
> endif
>
> config ARCH_NEEDS_CPU_IDLE_COUPLED
> Index: linux-2.6.git/drivers/cpuidle/Makefile
> ===================================================================
> --- linux-2.6.git.orig/drivers/cpuidle/Makefile 2019-06-13 16:19:27.878064316 -0400
> +++ linux-2.6.git/drivers/cpuidle/Makefile 2019-06-13 16:19:48.796544867 -0400
> @@ -7,6 +7,7 @@
> obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
> obj-$(CONFIG_DT_IDLE_STATES) += dt_idle_states.o
> obj-$(CONFIG_ARCH_HAS_CPU_RELAX) += poll_state.o
> +obj-$(CONFIG_HALTPOLL_CPUIDLE) += cpuidle-haltpoll.o
>
> ##################################################################################
> # ARM SoC drivers
> Index: linux-2.6.git/drivers/cpuidle/cpuidle-haltpoll.c
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.git/drivers/cpuidle/cpuidle-haltpoll.c 2019-06-13 18:41:39.305933413 -0400
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * cpuidle driver for haltpoll governor.
> + *
> + * Copyright 2019 Red Hat, Inc. and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + * Authors: Marcelo Tosatti <mtosatti@redhat.com>
> + */
> +
> +#include <linux/init.h>
> +#include <linux/cpuidle.h>
> +#include <linux/module.h>
> +#include <linux/sched/idle.h>
> +
> +static int default_enter_idle(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int index)
> +{
> + if (current_clr_polling_and_test()) {
> + local_irq_enable();
> + return index;
> + }
> + default_idle();
> + return index;
> +}
> +
> +static struct cpuidle_driver haltpoll_driver = {
> + .name = "haltpoll",
> + .owner = THIS_MODULE,
> + .states = {
> + { /* entry 0 is for polling */ },
> + {
> + .enter = default_enter_idle,
> + .exit_latency = 0,
> + .target_residency = 0,
On a second thought, I think that you need a target residency of at
least 1 (and an exit latency of at least 1) here to distinguish this
state from the polling one.
And if they both really were 0, polling wouldn't be needed at all ...
> + .power_usage = -1,
> + .name = "haltpoll idle",
> + .desc = "default architecture idle",
> + },
> + },
> + .safe_state_index = 0,
> + .state_count = 2,
> +};
> +
> +static int __init haltpoll_init(void)
> +{
> + struct cpuidle_driver *drv = &haltpoll_driver;
> +
> + cpuidle_poll_state_init(drv);
> +
> + return cpuidle_register(&haltpoll_driver, NULL);
> +}
> +
> +static void __exit haltpoll_exit(void)
> +{
> + cpuidle_unregister(&haltpoll_driver);
> +}
> +
> +module_init(haltpoll_init);
> +module_exit(haltpoll_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Marcelo Tosatti <mtosatti@redhat.com>");
> +
>
>
^ permalink raw reply
* Re: [PATCH RFC 1/2] PM / devfreq: Generic CPU frequency to device frequency mapping governor
From: Chanwoo Choi @ 2019-06-26 8:12 UTC (permalink / raw)
To: Sibi Sankar, Hsin-Yi Wang
Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Saravana Kannan, MyungJoo Ham, Kyungmin Park, Matthias Brugger,
linux-pm, linux-mediatek, lkml, Andrew-sh . Cheng,
linux-kernel-owner
In-Reply-To: <04bbd518efef8296e450e984e6afdba2@codeaurora.org>
Hello Sibi and Hsin-Yi,
On 19. 6. 20. 오후 6:41, Sibi Sankar wrote:
> Hey Hsin-Yi, Chanwoo
>
> On 2019-06-20 15:02, Hsin-Yi Wang wrote:
>> Hi Chanwoo Choi, Saravana Kannan and Sibi Sankar,
>>
>> I've also tested Sibi Sankar's patch[1] locally with mt8183-cci, and
>> it works fine too!
>> It'd be great if Sibi Sankar or anyone who is familiar with the
>> original design can finish this implementation. But if no one has time
>> to do that, I think I can also help on address the comments. Thanks!
>
> Now that we have a user :) I am happy
> to repost the patch with the comments
> addressed.
>
> https://lkml.org/lkml/2019/6/14/4
> Also with ^^ patch and few more in the
> series the dt parsing of required-opps
> should get further simplified.
Even if patch[1] suggested by Saravana is merged,
the patch[2] is necessary. Because, until now,
the child devfreq device cannot catch the timing
of CPU frequency without CPUFREQ notification.
The existing passive governor only supports between
devfreq device and other devfreq device.
[1] https://lkml.org/lkml/2019/6/14/4
[2]
[PATCH RFC 0/9] Add CPU based scaling support to Passive governor
- https://lore.kernel.org/lkml/08c3cff8c39e3d82e044db93e992da72@codeaurora.org/T/
[PATCH RFC 3/9] PM / devfreq: Add cpu based scaling support to passive_governor
- https://lore.kernel.org/lkml/08c3cff8c39e3d82e044db93e992da72@codeaurora.org/T/#m1cafb7baf687d2a680d39c85d3ec7d1b590b68fc
>
>>
>>
>> [1]
>> [RFC,2/9] OPP: Export a number of helpers to prevent code duplication
>> - https://patchwork.kernel.org/patch/10875199/
>> [RFC,3/9] PM / devfreq: Add cpu based scaling support to passive_governor
>> - https://patchwork.kernel.org/patch/10875195/
>>
>> Hsin-Yi
>>
>> On Thu, Jun 20, 2019 at 2:56 PM Chanwoo Choi <cw00.choi@samsung.com> wrote:
>>>
>>> + Sibi Sankar
>>>
>>> Hi, Hsin-Yi Wang, Saravana Kannan and Sibi Sankar
>>>
>>> I summarized the history of the related patch about this title.
>>>
>>> Firstly,
>>> As I knew, Saravana sent the patch[1] which contains
>>> 'governor_cpufreq_map.c' last year. According to the Myungoo's comment,
>>>
>>> Secondly,
>>> Sibi Sankar modified the 'governor_passive.c'[2] in order to support
>>> the mapping between cpu frequency and device frequency.
>>> Unfortunately, Sibi Sankar stopped the development about this
>>> because he had found the other method to get his purpose as I knew.
>>>
>>> Thirdly,
>>> Hsin-Yi Wang send the original patch of Saravana without modification.
>>>
>>>
>>> Sincerely, I think that the mapping between cpu frequency and device
>>> frequency is necessary. And I prefer the Sibi's approach which implements
>>> stuff to the existing 'passive' governor.
>>>
>>> We need to discuss about how to implement them by whom.
>>>
>>>
>>> [1] [v3,1/2] PM / devfreq: Generic CPU frequency to device frequency mapping governor
>>> - https://patchwork.kernel.org/patch/10553171/
>>>
>>> [2]
>>> [PATCH RFC 0/9] Add CPU based scaling support to Passive governor
>>> - https://lore.kernel.org/lkml/08c3cff8c39e3d82e044db93e992da72@codeaurora.org/T/
>>> [PATCH RFC 3/9] PM / devfreq: Add cpu based scaling support to passive_governor
>>> - https://lore.kernel.org/lkml/08c3cff8c39e3d82e044db93e992da72@codeaurora.org/T/#m1cafb7baf687d2a680d39c85d3ec7d1b590b68fc
>>>
>>>
>>> Best Regards,
>>> Chanwoo Choi
>>>
>>> On 19. 6. 18. 오후 1:14, Hsin-Yi Wang wrote:
>>> > From: Saravana Kannan <skannan@codeaurora.org>
>>> >
>>> > From: Saravana Kannan <skannan@codeaurora.org>
>>> >
>>> > Many CPU architectures have caches that can scale independent of the CPUs.
>>> > Frequency scaling of the caches is necessary to make sure the cache is not
>>> > a performance bottleneck that leads to poor performance and power. The same
>>> > idea applies for RAM/DDR.
>>> >
>>> > To achieve this, this patch adds a generic devfreq governor that takes the
>>> > current frequency of each CPU frequency domain and then adjusts the
>>> > frequency of the cache (or any devfreq device) based on the frequency of
>>> > the CPUs. It listens to CPU frequency transition notifiers to keep itself
>>> > up to date on the current CPU frequency.
>>> >
>>> > To decide the frequency of the device, the governor does one of the
>>> > following:
>>> >
>>> > * Uses a CPU frequency to device frequency mapping table
>>> > - Either one mapping table used for all CPU freq policies (typically used
>>> > for system with homogeneous cores/clusters that have the same OPPs).
>>> > - One mapping table per CPU freq policy (typically used for ASMP systems
>>> > with heterogeneous CPUs with different OPPs)
>>> >
>>> > OR
>>> >
>>> > * Scales the device frequency in proportion to the CPU frequency. So, if
>>> > the CPUs are running at their max frequency, the device runs at its max
>>> > frequency. If the CPUs are running at their min frequency, the device
>>> > runs at its min frequency. And interpolated for frequencies in between.
>>> >
>>> > Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
>>> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
>>> > ---
>>> > .../bindings/devfreq/devfreq-cpufreq-map.txt | 53 ++
>>> > drivers/devfreq/Kconfig | 8 +
>>> > drivers/devfreq/Makefile | 1 +
>>> > drivers/devfreq/governor_cpufreq_map.c | 583 ++++++++++++++++++
>>> > 4 files changed, 645 insertions(+)
>>> > create mode 100644 Documentation/devicetree/bindings/devfreq/devfreq-cpufreq-map.txt
>>> > create mode 100644 drivers/devfreq/governor_cpufreq_map.c
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/devfreq/devfreq-cpufreq-map.txt b/Documentation/devicetree/bindings/devfreq/devfreq-cpufreq-map.txt
>>> > new file mode 100644
>>> > index 000000000000..982a30bcfc86
>>> > --- /dev/null
>>> > +++ b/Documentation/devicetree/bindings/devfreq/devfreq-cpufreq-map.txt
>>> > @@ -0,0 +1,53 @@
>>> > +Devfreq CPUfreq governor
>>> > +
>>> > +devfreq-cpufreq-map is a parent device that contains one or more child devices.
>>> > +Each child device provides CPU frequency to device frequency mapping for a
>>> > +specific device. Examples of devices that could use this are: DDR, cache and
>>> > +CCI.
>>> > +
>>> > +Parent device name shall be "devfreq-cpufreq-map".
>>> > +
>>> > +Required child device properties:
>>> > +- cpu-to-dev-map, or cpu-to-dev-map-<X>:
>>> > + A list of tuples where each tuple consists of a
>>> > + CPU frequency (KHz) and the corresponding device
>>> > + frequency. CPU frequencies not listed in the table
>>> > + will use the device frequency that corresponds to the
>>> > + next rounded up CPU frequency.
>>> > + Use "cpu-to-dev-map" if all CPUs in the system should
>>> > + share same mapping.
>>> > + Use cpu-to-dev-map-<cpuid> to describe different
>>> > + mappings for different CPUs. The property should be
>>> > + listed only for the first CPU if multiple CPUs are
>>> > + synchronous.
>>> > +- target-dev: Phandle to device that this mapping applies to.
>>> > +
>>> > +Example:
>>> > + devfreq-cpufreq-map {
>>> > + cpubw-cpufreq {
>>> > + target-dev = <&cpubw>;
>>> > + cpu-to-dev-map =
>>> > + < 300000 1144000 >,
>>> > + < 422400 2288000 >,
>>> > + < 652800 3051000 >,
>>> > + < 883200 5996000 >,
>>> > + < 1190400 8056000 >,
>>> > + < 1497600 10101000 >,
>>> > + < 1728000 12145000 >,
>>> > + < 2649600 16250000 >;
>>> > + };
>>> > +
>>> > + cache-cpufreq {
>>> > + target-dev = <&cache>;
>>> > + cpu-to-dev-map =
>>> > + < 300000 300000 >,
>>> > + < 422400 422400 >,
>>> > + < 652800 499200 >,
>>> > + < 883200 576000 >,
>>> > + < 960000 960000 >,
>>> > + < 1497600 1036800 >,
>>> > + < 1574400 1574400 >,
>>> > + < 1728000 1651200 >,
>>> > + < 2649600 1728000 >;
>>> > + };
>>> > + };
>>> > diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>>> > index 0c8204d6b78a..0303f5a400b6 100644
>>> > --- a/drivers/devfreq/Kconfig
>>> > +++ b/drivers/devfreq/Kconfig
>>> > @@ -74,6 +74,14 @@ config DEVFREQ_GOV_PASSIVE
>>> > through sysfs entries. The passive governor recommends that
>>> > devfreq device uses the OPP table to get the frequency/voltage.
>>> >
>>> > +config DEVFREQ_GOV_CPUFREQ_MAP
>>> > + tristate "CPUfreq Map"
>>> > + depends on CPU_FREQ
>>> > + help
>>> > + Chooses frequency based on the online CPUs' current frequency and a
>>> > + CPU frequency to device frequency mapping table(s). This governor
>>> > + can be useful for controlling devices such as DDR, cache, CCI, etc.
>>> > +
>>> > comment "DEVFREQ Drivers"
>>> >
>>> > config ARM_EXYNOS_BUS_DEVFREQ
>>> > diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>>> > index 817dde779f16..81141e2c784f 100644
>>> > --- a/drivers/devfreq/Makefile
>>> > +++ b/drivers/devfreq/Makefile
>>> > @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE) += governor_performance.o
>>> > obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE) += governor_powersave.o
>>> > obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
>>> > obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o
>>> > +obj-$(CONFIG_DEVFREQ_GOV_CPUFREQ_MAP) += governor_cpufreq_map.o
>>> >
>>> > # DEVFREQ Drivers
>>> > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
>>> > diff --git a/drivers/devfreq/governor_cpufreq_map.c b/drivers/devfreq/governor_cpufreq_map.c
>>> > new file mode 100644
>>> > index 000000000000..084a3ffb8f54
>>> > --- /dev/null
>>> > +++ b/drivers/devfreq/governor_cpufreq_map.c
>>> > @@ -0,0 +1,583 @@
>>> > +// SPDX-License-Identifier: GPL-2.0
>>> > +/*
>>> > + * Copyright (c) 2014-2015, 2018, The Linux Foundation. All rights reserved.
>>> > + */
>>> > +
>>> > +#define pr_fmt(fmt) "dev-cpufreq-map: " fmt
>>> > +
>>> > +#include <linux/devfreq.h>
>>> > +#include <linux/cpu.h>
>>> > +#include <linux/cpufreq.h>
>>> > +#include <linux/cpumask.h>
>>> > +#include <linux/slab.h>
>>> > +#include <linux/platform_device.h>
>>> > +#include <linux/of.h>
>>> > +#include <linux/module.h>
>>> > +#include "governor.h"
>>> > +
>>> > +struct cpu_state {
>>> > + unsigned int freq;
>>> > + unsigned int min_freq;
>>> > + unsigned int max_freq;
>>> > + unsigned int first_cpu;
>>> > +};
>>> > +static struct cpu_state *state[NR_CPUS];
>>> > +static int cpufreq_cnt;
>>> > +
>>> > +struct freq_map {
>>> > + unsigned int cpu_khz;
>>> > + unsigned int target_freq;
>>> > +};
>>> > +
>>> > +struct devfreq_node {
>>> > + struct devfreq *df;
>>> > + void *orig_data;
>>> > + struct device *dev;
>>> > + struct device_node *of_node;
>>> > + struct list_head list;
>>> > + struct freq_map **map;
>>> > + struct freq_map *common_map;
>>> > +};
>>> > +static LIST_HEAD(devfreq_list);
>>> > +static DEFINE_MUTEX(state_lock);
>>> > +static DEFINE_MUTEX(cpufreq_reg_lock);
>>> > +
>>> > +static void update_all_devfreqs(void)
>>> > +{
>>> > + struct devfreq_node *node;
>>> > +
>>> > + list_for_each_entry(node, &devfreq_list, list) {
>>> > + struct devfreq *df = node->df;
>>> > +
>>> > + if (!node->df)
>>> > + continue;
>>> > + mutex_lock(&df->lock);
>>> > + update_devfreq(df);
>>> > + mutex_unlock(&df->lock);
>>> > +
>>> > + }
>>> > +}
>>> > +
>>> > +static struct devfreq_node *find_devfreq_node(struct device *dev)
>>> > +{
>>> > + struct devfreq_node *node;
>>> > +
>>> > + list_for_each_entry(node, &devfreq_list, list)
>>> > + if (node->dev == dev || node->of_node == dev->of_node)
>>> > + return node;
>>> > +
>>> > + return NULL;
>>> > +}
>>> > +
>>> > +/* ==================== cpufreq part ==================== */
>>> > +static struct cpu_state *add_policy(struct cpufreq_policy *policy)
>>> > +{
>>> > + struct cpu_state *new_state;
>>> > + unsigned int cpu, first_cpu;
>>> > +
>>> > + new_state = kzalloc(sizeof(struct cpu_state), GFP_KERNEL);
>>> > + if (!new_state)
>>> > + return NULL;
>>> > +
>>> > + first_cpu = cpumask_first(policy->related_cpus);
>>> > + new_state->first_cpu = first_cpu;
>>> > + new_state->freq = policy->cur;
>>> > + new_state->min_freq = policy->cpuinfo.min_freq;
>>> > + new_state->max_freq = policy->cpuinfo.max_freq;
>>> > +
>>> > + for_each_cpu(cpu, policy->related_cpus)
>>> > + state[cpu] = new_state;
>>> > +
>>> > + return new_state;
>>> > +}
>>> > +
>>> > +static int cpufreq_trans_notifier(struct notifier_block *nb,
>>> > + unsigned long event, void *data)
>>> > +{
>>> > + struct cpufreq_freqs *freq = data;
>>> > + struct cpu_state *s;
>>> > + struct cpufreq_policy *policy = NULL;
>>> > +
>>> > + if (event != CPUFREQ_POSTCHANGE)
>>> > + return 0;
>>> > +
>>> > + mutex_lock(&state_lock);
>>> > +
>>> > + s = state[freq->cpu];
>>> > + if (!s) {
>>> > + policy = cpufreq_cpu_get(freq->cpu);
>>> > + if (policy) {
>>> > + s = add_policy(policy);
>>> > + cpufreq_cpu_put(policy);
>>> > + }
>>> > + }
>>> > + if (!s)
>>> > + goto out;
>>> > +
>>> > + if (s->freq != freq->new || policy) {
>>> > + s->freq = freq->new;
>>> > + update_all_devfreqs();
>>> > + }
>>> > +
>>> > +out:
>>> > + mutex_unlock(&state_lock);
>>> > + return 0;
>>> > +}
>>> > +
>>> > +static struct notifier_block cpufreq_trans_nb = {
>>> > + .notifier_call = cpufreq_trans_notifier
>>> > +};
>>> > +
>>> > +static int register_cpufreq(void)
>>> > +{
>>> > + int ret = 0;
>>> > + unsigned int cpu;
>>> > + struct cpufreq_policy *policy;
>>> > +
>>> > + mutex_lock(&cpufreq_reg_lock);
>>> > +
>>> > + if (cpufreq_cnt)
>>> > + goto cnt_not_zero;
>>> > +
>>> > + get_online_cpus();
>>> > + ret = cpufreq_register_notifier(&cpufreq_trans_nb,
>>> > + CPUFREQ_TRANSITION_NOTIFIER);
>>> > + if (ret)
>>> > + goto out;
>>> > +
>>> > + for_each_online_cpu(cpu) {
>>> > + policy = cpufreq_cpu_get(cpu);
>>> > + if (policy) {
>>> > + add_policy(policy);
>>> > + cpufreq_cpu_put(policy);
>>> > + }
>>> > + }
>>> > +out:
>>> > + put_online_cpus();
>>> > +cnt_not_zero:
>>> > + if (!ret)
>>> > + cpufreq_cnt++;
>>> > + mutex_unlock(&cpufreq_reg_lock);
>>> > + return ret;
>>> > +}
>>> > +
>>> > +static int unregister_cpufreq(void)
>>> > +{
>>> > + int ret = 0;
>>> > + int cpu;
>>> > +
>>> > + mutex_lock(&cpufreq_reg_lock);
>>> > +
>>> > + if (cpufreq_cnt > 1)
>>> > + goto out;
>>> > +
>>> > + cpufreq_unregister_notifier(&cpufreq_trans_nb,
>>> > + CPUFREQ_TRANSITION_NOTIFIER);
>>> > +
>>> > + for (cpu = ARRAY_SIZE(state) - 1; cpu >= 0; cpu--) {
>>> > + if (!state[cpu])
>>> > + continue;
>>> > + if (state[cpu]->first_cpu == cpu)
>>> > + kfree(state[cpu]);
>>> > + state[cpu] = NULL;
>>> > + }
>>> > +
>>> > +out:
>>> > + cpufreq_cnt--;
>>> > + mutex_unlock(&cpufreq_reg_lock);
>>> > + return ret;
>>> > +}
>>> > +
>>> > +/* ==================== devfreq part ==================== */
>>> > +
>>> > +static unsigned int interpolate_freq(struct devfreq *df, unsigned int cpu)
>>> > +{
>>> > + unsigned long *freq_table = df->profile->freq_table;
>>> > + unsigned int cpu_min = state[cpu]->min_freq;
>>> > + unsigned int cpu_max = state[cpu]->max_freq;
>>> > + unsigned int cpu_freq = state[cpu]->freq;
>>> > + unsigned int dev_min, dev_max, cpu_percent;
>>> > +
>>> > + if (freq_table) {
>>> > + dev_min = freq_table[0];
>>> > + dev_max = freq_table[df->profile->max_state - 1];
>>> > + } else {
>>> > + if (df->max_freq <= df->min_freq)
>>> > + return 0;
>>> > + dev_min = df->min_freq;
>>> > + dev_max = df->max_freq;
>>> > + }
>>> > +
>>> > + cpu_percent = ((cpu_freq - cpu_min) * 100) / (cpu_max - cpu_min);
>>> > + return dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
>>> > +}
>>> > +
>>> > +static unsigned int cpu_to_dev_freq(struct devfreq *df, unsigned int cpu)
>>> > +{
>>> > + struct freq_map *map = NULL;
>>> > + unsigned int cpu_khz = 0, freq;
>>> > + struct devfreq_node *n = df->data;
>>> > +
>>> > + if (!state[cpu] || state[cpu]->first_cpu != cpu) {
>>> > + freq = 0;
>>> > + goto out;
>>> > + }
>>> > +
>>> > + if (n->common_map)
>>> > + map = n->common_map;
>>> > + else if (n->map)
>>> > + map = n->map[cpu];
>>> > +
>>> > + cpu_khz = state[cpu]->freq;
>>> > +
>>> > + if (!map) {
>>> > + freq = interpolate_freq(df, cpu);
>>> > + goto out;
>>> > + }
>>> > +
>>> > + while (map->cpu_khz && map->cpu_khz < cpu_khz)
>>> > + map++;
>>> > + if (!map->cpu_khz)
>>> > + map--;
>>> > + freq = map->target_freq;
>>> > +
>>> > +out:
>>> > + dev_dbg(df->dev.parent, "CPU%u: %d -> dev: %u\n", cpu, cpu_khz, freq);
>>> > + return freq;
>>> > +}
>>> > +
>>> > +static int devfreq_cpufreq_get_freq(struct devfreq *df,
>>> > + unsigned long *freq)
>>> > +{
>>> > + unsigned int cpu, tgt_freq = 0;
>>> > + struct devfreq_node *node;
>>> > +
>>> > + node = df->data;
>>> > + if (!node) {
>>> > + pr_err("Unable to find devfreq node!\n");
>>> > + return -ENODEV;
>>> > + }
>>> > +
>>> > + for_each_possible_cpu(cpu)
>>> > + tgt_freq = max(tgt_freq, cpu_to_dev_freq(df, cpu));
>>> > +
>>> > + *freq = tgt_freq;
>>> > + return 0;
>>> > +}
>>> > +
>>> > +static unsigned int show_table(char *buf, unsigned int len,
>>> > + struct freq_map *map)
>>> > +{
>>> > + unsigned int cnt = 0;
>>> > +
>>> > + cnt += snprintf(buf + cnt, len - cnt, "CPU freq\tDevice freq\n");
>>> > +
>>> > + while (map->cpu_khz && cnt < len) {
>>> > + cnt += snprintf(buf + cnt, len - cnt, "%8u\t%11u\n",
>>> > + map->cpu_khz, map->target_freq);
>>> > + map++;
>>> > + }
>>> > + if (cnt < len)
>>> > + cnt += snprintf(buf + cnt, len - cnt, "\n");
>>> > +
>>> > + return cnt;
>>> > +}
>>> > +
>>> > +static ssize_t freq_map_show(struct device *dev, struct device_attribute *attr,
>>> > + char *buf)
>>> > +{
>>> > + struct devfreq *df = to_devfreq(dev);
>>> > + struct devfreq_node *n = df->data;
>>> > + struct freq_map *map;
>>> > + unsigned int cnt = 0, cpu;
>>> > +
>>> > + mutex_lock(&state_lock);
>>> > + if (n->common_map) {
>>> > + map = n->common_map;
>>> > + cnt += snprintf(buf + cnt, PAGE_SIZE - cnt,
>>> > + "Common table for all CPUs:\n");
>>> > + cnt += show_table(buf + cnt, PAGE_SIZE - cnt, map);
>>> > + } else if (n->map) {
>>> > + for_each_possible_cpu(cpu) {
>>> > + map = n->map[cpu];
>>> > + if (!map)
>>> > + continue;
>>> > + cnt += snprintf(buf + cnt, PAGE_SIZE - cnt,
>>> > + "CPU %u:\n", cpu);
>>> > + if (cnt >= PAGE_SIZE)
>>> > + break;
>>> > + cnt += show_table(buf + cnt, PAGE_SIZE - cnt, map);
>>> > + if (cnt >= PAGE_SIZE)
>>> > + break;
>>> > + }
>>> > + } else {
>>> > + cnt += snprintf(buf + cnt, PAGE_SIZE - cnt,
>>> > + "Device freq interpolated based on CPU freq\n");
>>> > + }
>>> > + mutex_unlock(&state_lock);
>>> > +
>>> > + return cnt;
>>> > +}
>>> > +
>>> > +static DEVICE_ATTR_RO(freq_map);
>>> > +static struct attribute *dev_attr[] = {
>>> > + &dev_attr_freq_map.attr,
>>> > + NULL,
>>> > +};
>>> > +
>>> > +static struct attribute_group dev_attr_group = {
>>> > + .name = "cpufreq-map",
>>> > + .attrs = dev_attr,
>>> > +};
>>> > +
>>> > +static int devfreq_cpufreq_gov_start(struct devfreq *devfreq)
>>> > +{
>>> > + int ret = 0;
>>> > + struct devfreq_node *node;
>>> > + bool alloc = false;
>>> > +
>>> > + ret = register_cpufreq();
>>> > + if (ret)
>>> > + return ret;
>>> > +
>>> > + ret = sysfs_create_group(&devfreq->dev.kobj, &dev_attr_group);
>>> > + if (ret) {
>>> > + unregister_cpufreq();
>>> > + return ret;
>>> > + }
>>> > +
>>> > + mutex_lock(&state_lock);
>>> > +
>>> > + node = find_devfreq_node(devfreq->dev.parent);
>>> > + if (node == NULL) {
>>> > + node = kzalloc(sizeof(struct devfreq_node), GFP_KERNEL);
>>> > + if (!node) {
>>> > + ret = -ENOMEM;
>>> > + goto alloc_fail;
>>> > + }
>>> > + alloc = true;
>>> > + node->dev = devfreq->dev.parent;
>>> > + list_add_tail(&node->list, &devfreq_list);
>>> > + }
>>> > + node->df = devfreq;
>>> > + node->orig_data = devfreq->data;
>>> > + devfreq->data = node;
>>> > +
>>> > + mutex_lock(&devfreq->lock);
>>> > + ret = update_devfreq(devfreq);
>>> > + mutex_unlock(&devfreq->lock);
>>> > + if (ret) {
>>> > + pr_err("Freq update failed!\n");
>>> > + goto update_fail;
>>> > + }
>>> > +
>>> > + mutex_unlock(&state_lock);
>>> > + return 0;
>>> > +
>>> > +update_fail:
>>> > + devfreq->data = node->orig_data;
>>> > + if (alloc) {
>>> > + list_del(&node->list);
>>> > + kfree(node);
>>> > + }
>>> > +alloc_fail:
>>> > + mutex_unlock(&state_lock);
>>> > + sysfs_remove_group(&devfreq->dev.kobj, &dev_attr_group);
>>> > + unregister_cpufreq();
>>> > + return ret;
>>> > +}
>>> > +
>>> > +static void devfreq_cpufreq_gov_stop(struct devfreq *devfreq)
>>> > +{
>>> > + struct devfreq_node *node = devfreq->data;
>>> > +
>>> > + mutex_lock(&state_lock);
>>> > + devfreq->data = node->orig_data;
>>> > + if (node->map || node->common_map) {
>>> > + node->df = NULL;
>>> > + } else {
>>> > + list_del(&node->list);
>>> > + kfree(node);
>>> > + }
>>> > + mutex_unlock(&state_lock);
>>> > +
>>> > + sysfs_remove_group(&devfreq->dev.kobj, &dev_attr_group);
>>> > + unregister_cpufreq();
>>> > +}
>>> > +
>>> > +static int devfreq_cpufreq_ev_handler(struct devfreq *devfreq,
>>> > + unsigned int event, void *data)
>>> > +{
>>> > + int ret;
>>> > +
>>> > + switch (event) {
>>> > + case DEVFREQ_GOV_START:
>>> > +
>>> > + ret = devfreq_cpufreq_gov_start(devfreq);
>>> > + if (ret) {
>>> > + pr_err("Governor start failed!\n");
>>> > + return ret;
>>> > + }
>>> > + pr_debug("Enabled CPUfreq-map governor\n");
>>> > + break;
>>> > +
>>> > + case DEVFREQ_GOV_STOP:
>>> > +
>>> > + devfreq_cpufreq_gov_stop(devfreq);
>>> > + pr_debug("Disabled dev CPUfreq-map governor\n");
>>> > + break;
>>> > + }
>>> > +
>>> > + return 0;
>>> > +}
>>> > +
>>> > +static struct devfreq_governor devfreq_cpufreq = {
>>> > + .name = "cpufreq-map",
>>> > + .get_target_freq = devfreq_cpufreq_get_freq,
>>> > + .event_handler = devfreq_cpufreq_ev_handler,
>>> > +};
>>> > +
>>> > +#define NUM_COLS 2
>>> > +static struct freq_map *read_tbl(struct device_node *of_node, char *prop_name)
>>> > +{
>>> > + int len, nf, i, j;
>>> > + u32 data;
>>> > + struct freq_map *tbl;
>>> > +
>>> > + if (!of_find_property(of_node, prop_name, &len))
>>> > + return NULL;
>>> > + len /= sizeof(data);
>>> > +
>>> > + if (len % NUM_COLS || len == 0)
>>> > + return NULL;
>>> > + nf = len / NUM_COLS;
>>> > +
>>> > + tbl = kzalloc((nf + 1) * sizeof(*tbl), GFP_KERNEL);
>>> > + if (!tbl)
>>> > + return NULL;
>>> > +
>>> > + for (i = 0, j = 0; i < nf; i++, j += 2) {
>>> > + of_property_read_u32_index(of_node, prop_name, j, &data);
>>> > + tbl[i].cpu_khz = data;
>>> > +
>>> > + of_property_read_u32_index(of_node, prop_name, j + 1, &data);
>>> > + tbl[i].target_freq = data;
>>> > + }
>>> > + tbl[i].cpu_khz = 0;
>>> > +
>>> > + return tbl;
>>> > +}
>>> > +
>>> > +#define PROP_TARGET "target-dev"
>>> > +#define PROP_TABLE "cpu-to-dev-map"
>>> > +static int add_table_from_of(struct device_node *of_node)
>>> > +{
>>> > + struct device_node *target_of_node;
>>> > + struct devfreq_node *node;
>>> > + struct freq_map *common_tbl;
>>> > + struct freq_map **tbl_list = NULL;
>>> > + static char prop_name[] = PROP_TABLE "-999999";
>>> > + int cpu, ret, cnt = 0, prop_sz = ARRAY_SIZE(prop_name);
>>> > +
>>> > + target_of_node = of_parse_phandle(of_node, PROP_TARGET, 0);
>>> > + if (!target_of_node)
>>> > + return -EINVAL;
>>> > +
>>> > + node = kzalloc(sizeof(struct devfreq_node), GFP_KERNEL);
>>> > + if (!node)
>>> > + return -ENOMEM;
>>> > +
>>> > + common_tbl = read_tbl(of_node, PROP_TABLE);
>>> > + if (!common_tbl) {
>>> > + tbl_list = kzalloc(sizeof(*tbl_list) * NR_CPUS, GFP_KERNEL);
>>> > + if (!tbl_list) {
>>> > + ret = -ENOMEM;
>>> > + goto err_list;
>>> > + }
>>> > +
>>> > + for_each_possible_cpu(cpu) {
>>> > + ret = snprintf(prop_name, prop_sz, "%s-%d",
>>> > + PROP_TABLE, cpu);
>>> > + if (ret >= prop_sz) {
>>> > + pr_warn("More CPUs than I can handle!\n");
>>> > + pr_warn("Skipping rest of the tables!\n");
>>> > + break;
>>> > + }
>>> > + tbl_list[cpu] = read_tbl(of_node, prop_name);
>>> > + if (tbl_list[cpu])
>>> > + cnt++;
>>> > + }
>>> > + }
>>> > + if (!common_tbl && !cnt) {
>>> > + ret = -EINVAL;
>>> > + goto err_tbl;
>>> > + }
>>> > +
>>> > + mutex_lock(&state_lock);
>>> > + node->of_node = target_of_node;
>>> > + node->map = tbl_list;
>>> > + node->common_map = common_tbl;
>>> > + list_add_tail(&node->list, &devfreq_list);
>>> > + mutex_unlock(&state_lock);
>>> > +
>>> > + return 0;
>>> > +err_tbl:
>>> > + kfree(tbl_list);
>>> > +err_list:
>>> > + kfree(node);
>>> > + return ret;
>>> > +}
>>> > +
>>> > +static int __init devfreq_cpufreq_init(void)
>>> > +{
>>> > + int ret;
>>> > + struct device_node *of_par, *of_child;
>>> > +
>>> > + of_par = of_find_node_by_name(NULL, "devfreq-cpufreq-map");
>>> > + if (of_par) {
>>> > + for_each_child_of_node(of_par, of_child) {
>>> > + ret = add_table_from_of(of_child);
>>> > + if (ret)
>>> > + pr_err("Parsing %s failed!\n", of_child->name);
>>> > + else
>>> > + pr_debug("Parsed %s.\n", of_child->name);
>>> > + }
>>> > + of_node_put(of_par);
>>> > + } else {
>>> > + pr_info("No tables parsed from DT.\n");
>>> > + }
>>> > +
>>> > + ret = devfreq_add_governor(&devfreq_cpufreq);
>>> > + if (ret) {
>>> > + pr_err("cpufreq-map governor add failed!\n");
>>> > + return ret;
>>> > + }
>>> > +
>>> > + return 0;
>>> > +}
>>> > +subsys_initcall(devfreq_cpufreq_init);
>>> > +
>>> > +static void __exit devfreq_cpufreq_exit(void)
>>> > +{
>>> > + int ret, cpu;
>>> > + struct devfreq_node *node, *tmp;
>>> > +
>>> > + ret = devfreq_remove_governor(&devfreq_cpufreq);
>>> > + if (ret)
>>> > + pr_err("cpufreq-map governor remove failed!\n");
>>> > +
>>> > + mutex_lock(&state_lock);
>>> > + list_for_each_entry_safe(node, tmp, &devfreq_list, list) {
>>> > + kfree(node->common_map);
>>> > + for_each_possible_cpu(cpu)
>>> > + kfree(node->map[cpu]);
>>> > + kfree(node->map);
>>> > + list_del(&node->list);
>>> > + kfree(node);
>>> > + }
>>> > + mutex_unlock(&state_lock);
>>> > +}
>>> > +module_exit(devfreq_cpufreq_exit);
>>> > +
>>> > +MODULE_DESCRIPTION("devfreq gov that sets dev freq based on current CPU freq");
>>> > +MODULE_LICENSE("GPL v2");
>>> >
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* [GIT PULL] OPP changes for 5.3
From: Viresh Kumar @ 2019-06-26 7:20 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PM
Hi Rafael,
This pull request contains:
- OPP core changes to support a wider range of devices, like IO
devices (Rajendra Nayak and Stehpen Boyd).
- Fixes around genpd_virt_devs (Viresh Kumar).
- Fix for platform with set_opp() callback (Dmitry Osipenko).
--
viresh
-------------------------8<-------------------------
The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9:
Linux 5.2-rc1 (2019-05-19 15:47:09 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/linux-next
for you to fetch changes up to 560d1bcad715c215e7ffe5d7cffe045974b623d0:
opp: Don't use IS_ERR on invalid supplies (2019-06-24 12:48:29 +0530)
----------------------------------------------------------------
Dmitry Osipenko (1):
opp: Don't use IS_ERR on invalid supplies
Rajendra Nayak (1):
opp: Make dev_pm_opp_set_rate() handle freq = 0 to drop performance votes
Stephen Boyd (1):
opp: Don't overwrite rounded clk rate
Viresh Kumar (2):
opp: Attach genpds to devices from within OPP core
opp: Allocate genpd_virt_devs from dev_pm_opp_attach_genpd()
drivers/opp/core.c | 176 ++++++++++++++++++++++++++++++++-----------------
drivers/opp/of.c | 30 +--------
include/linux/pm_opp.h | 8 +--
3 files changed, 122 insertions(+), 92 deletions(-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox