* Re: [PATCH v1 2/3] OPP: Add function to look up required OPP's for a given OPP
From: Chanwoo Choi @ 2019-06-23 4:27 UTC (permalink / raw)
To: Saravana Kannan
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Android Kernel Team, Linux PM list, linux-kernel
In-Reply-To: <CAGETcx9Gi24bng_PCqc6=9S584va4hRc4HHZtBLevKHgYGSNDA@mail.gmail.com>
Hi,
2019년 6월 23일 (일) 오전 6:42, Saravana Kannan <saravanak@google.com>님이 작성:
>
> On Sat, Jun 22, 2019 at 4:50 AM Chanwoo Choi <cwchoi00@gmail.com> wrote:
> >
> > Hi,
> >
> > Absolutely, I like this approach. I think that it is necessary to make
> > the connection
> > between frequencies of devices.
>
> Happy to hear that.
>
> > But, I have a question on below.
> >
> > 2019년 6월 22일 (토) 오전 9:35, Saravana Kannan <saravanak@google.com>님이 작성:
> > >
> > > Add a function that allows looking up required OPPs given a source OPP
> > > table, destination OPP table and the source OPP.
> > >
> > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > ---
> > > drivers/opp/core.c | 54 ++++++++++++++++++++++++++++++++++++++++++
> > > include/linux/pm_opp.h | 11 +++++++++
> > > 2 files changed, 65 insertions(+)
> > >
> > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> > > index 74c7bdc6f463..4f7870bffbf8 100644
> > > --- a/drivers/opp/core.c
> > > +++ b/drivers/opp/core.c
> > > @@ -1830,6 +1830,60 @@ void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table,
> > > dev_err(virt_dev, "Failed to find required device entry\n");
> > > }
> > >
> > > +/**
> > > + * dev_pm_opp_xlate_opp() - Find required OPP for src_table OPP.
> > > + * @src_table: OPP table which has dst_table as one of its required OPP table.
> > > + * @dst_table: Required OPP table of the src_table.
> > > + * @pstate: OPP of the src_table.
> > > + *
> > > + * This function returns the OPP (present in @dst_table) pointed out by the
> > > + * "required-opps" property of the OPP (present in @src_table).
> > > + *
> > > + * The callers are required to call dev_pm_opp_put() for the returned OPP after
> > > + * use.
> > > + *
> > > + * Return: destination table OPP on success, otherwise NULL on errors.
> > > + */
> > > +struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
> > > + struct opp_table *dst_table,
> > > + struct dev_pm_opp *src_opp)
> > > +{
> > > + struct dev_pm_opp *opp, *dest_opp = NULL;
> > > + int i;
> > > +
> > > + if (!src_table || !dst_table || !src_opp)
> > > + return NULL;
> > > +
> > > + for (i = 0; i < src_table->required_opp_count; i++) {
> > > + if (src_table->required_opp_tables[i]->np == dst_table->np)
> > > + break;
> > > + }
> > > +
> > > + if (unlikely(i == src_table->required_opp_count)) {
> > > + pr_err("%s: Couldn't find matching OPP table (%p: %p)\n",
> > > + __func__, src_table, dst_table);
> > > + return NULL;
> > > + }
> > > +
> > > + mutex_lock(&src_table->lock);
> > > +
> > > + list_for_each_entry(opp, &src_table->opp_list, node) {
> > > + if (opp == src_opp) {
> > > + dest_opp = opp->required_opps[i];
> >
> > Correct me if I am wrong. This patch assume that 'i' index is same on between
> > [1] and [2]. But in order to guarantee this assumption, all OPP entries
> > in the same opp_table have to have the same number of 'required-opps' properties
> > and keep the sequence among 'required-opps' entries.
> >
> > [1] src_table->required_opp_tables[i]->np
> > [2] opp->required_opps[I];
> >
> > For example, three OPP entries in the 'parent_bus_opp'
> > have the different sequence of 'required-opps' and the different
> > number of 'required-opps'. Is it no problem?
> >
> > parent_bus_opp: opp_table {
> > compatible = "operating-points-v2";
> >
> > opp2 {
> > opp-hz = /bits/ 64 <200000>;
> > required-opps = <&child_bus_a_opp2>, <&child_bus_b_opp2>,
> > <&child_bus_c_opp2>;
> > };
> >
> > opp1 {
> > opp-hz = /bits/ 64 <200000>;
> > // change the sequence between child_bus_b_opp2 and child_bus_c_opp2
> > required-opps = <&child_bus_a_opp2>, <&child_bus_c_opp2>,
> > <&child_bus_b_opp2>
> > };
> >
> > opp0 {
> > opp-hz = /bits/ 64 <200000>;
> > // missing 'child_bus_a_opp2'
> > required-opps = <&child_bus_c_opp2>, <&child_bus_b_opp2>
> > };
> >
> > }
> >
>
> I get your question. If I'm not mistaken the OPP framework DT parsing
> code makes the assumption that the required-opps list has the phandles
> in the same order for each "row" in the OPP table. It actually only
> looks at the first OPP entry to figure out the list of required OPP
> tables.
Thanks for description. It is the limitation of 'required-opps' until now.
>
> Technically one can write code to deal with random order of the
> required-opp list, but doesn't seem like that's worth it because
> there's no need to have that order all mixed up in DT. And even if
> someone wants to add support for that, I don't think improving the DT
> parsing to handle random order would be part of this patch series.
I understand the existing ' required-opps' only consider the same sequence
of entries which are included in the same OPP table.
One more thing, 'required-opps' properties doesn't support
the other OPP enters of the different OPP table. Is it right of 'required-opps'?
Except for the random order, just each OPP might will requires
the different 'required-opps' of different OPP table. Even if it is
not related to random order, I think that this approach cannot
support them.
For example as following:
- opp2 used the OPP entries of 'child_bus_A' and 'child_bus_B' opp-table.
- opp1 used the OPP entries of 'child_bus_C' and 'child_bus_D' opp-table.
parent_bus_opp: opp_table {
compatible = "operating-points-v2";
opp2 {
opp-hz = /bits/ 64 <200000>;
required-opps = <&child_bus_A_opp2>, <&child_bus_B_opp2>;
};
opp1 {
opp-hz = /bits/ 64 <200000>;
required-opps = <&child_bus_C_opp0>, <&child_bus_D_opp0>;
};
};
--
Best Regards,
Chanwoo Choi
^ permalink raw reply
* Re: [PATCH V34 10/29] hibernate: Disable when the kernel is locked down
From: Kees Cook @ 2019-06-22 23:55 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Josh Boyer, David Howells, Matthew Garrett, rjw, pavel, linux-pm
In-Reply-To: <20190622000358.19895-11-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:39PM -0700, Matthew Garrett wrote:
> From: Josh Boyer <jwboyer@fedoraproject.org>
>
> There is currently no way to verify the resume image when returning
> from hibernate. This might compromise the signed modules trust model,
> so until we can work with signed hibernate images we disable it when the
> kernel is locked down.
>
> Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: rjw@rjwysocki.net
> Cc: pavel@ucw.cz
> cc: linux-pm@vger.kernel.org
> ---
> include/linux/security.h | 1 +
> kernel/power/hibernate.c | 3 ++-
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 00a31ab2e5ba..a051f21a1144 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -85,6 +85,7 @@ enum lockdown_reason {
> LOCKDOWN_MODULE_SIGNATURE,
> LOCKDOWN_DEV_MEM,
> LOCKDOWN_KEXEC,
> + LOCKDOWN_HIBERNATION,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index abef759de7c8..3a9cb2d3da4a 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -32,6 +32,7 @@
> #include <linux/ctype.h>
> #include <linux/genhd.h>
> #include <linux/ktime.h>
> +#include <linux/security.h>
> #include <trace/events/power.h>
>
> #include "power.h"
> @@ -70,7 +71,7 @@ static const struct platform_hibernation_ops *hibernation_ops;
>
> bool hibernation_available(void)
> {
> - return (nohibernate == 0);
> + return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
> }
>
> /**
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 08fcd8116db3..ce5b3da9bd09 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -21,6 +21,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
> [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
> [LOCKDOWN_KEXEC] = "kexec of unsigned images",
> + [LOCKDOWN_HIBERNATION] = "hibernation",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v1 3/3] PM / devfreq: Add required OPPs support to passive governor
From: Saravana Kannan @ 2019-06-22 21:45 UTC (permalink / raw)
To: cwchoi00
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Android Kernel Team, Linux PM list, linux-kernel
In-Reply-To: <CAGTfZH1COWzhDBr63S18md44ypKixstHHsX7cm5LnAhXXbfecA@mail.gmail.com>
On Sat, Jun 22, 2019 at 5:01 AM Chanwoo Choi <cwchoi00@gmail.com> wrote:
>
> Hi,
>
> Absolutely, I agree this approach.
Thanks!
> But, I add some comments on below. please check them.
>
> 2019년 6월 22일 (토) 오전 9:36, Saravana Kannan <saravanak@google.com>님이 작성:
> >
> > Look at the required OPPs of the "parent" device to determine the OPP that
> > is required from the slave device managed by the passive governor. This
> > allows having mappings between a parent device and a slave device even when
> > they don't have the same number of OPPs.
> >
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > ---
> > drivers/devfreq/governor_passive.c | 25 +++++++++++++++++++++++--
> > 1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
> > index 3bc29acbd54e..bd4a98bb15b1 100644
> > --- a/drivers/devfreq/governor_passive.c
> > +++ b/drivers/devfreq/governor_passive.c
> > @@ -21,8 +21,9 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> > struct devfreq_passive_data *p_data
> > = (struct devfreq_passive_data *)devfreq->data;
> > struct devfreq *parent_devfreq = (struct devfreq *)p_data->parent;
> > + struct opp_table *opp_table = NULL, *c_opp_table = NULL;
>
> In this function, the base device is passive devfreq device.
> So, I think that better to define the 'parent_opp_table' instead of 'opp_table'
> indicating the OPP table of parent devfreq device. And better to define
> just 'opp_table' instead of 'c_opp_table' indicating the passive devfreq device.
> - opp_table -> parent_opp_table
> - c_opp_table -> opp_table
Sounds good. I did it that way at first, but I wanted to keep the diff
simple in my first patch series. So renamed it. I can do the rename
that you suggested. Makes sense to me.
> > unsigned long child_freq = ULONG_MAX;
> > - struct dev_pm_opp *opp;
> > + struct dev_pm_opp *opp = NULL, *c_opp = NULL;
>
> Ditto. I think that better to define the variables as following:
> - opp -> parent_opp
> - c_cpp -> opp
Will do.
>
> > int i, count, ret = 0;
> >
> > /*
> > @@ -65,7 +66,20 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> > goto out;
> > }
> >
> > - dev_pm_opp_put(opp);
> > + opp_table = dev_pm_opp_get_opp_table(parent_devfreq->dev.parent);
>
> devfreq_passive_get_target_freq() is called frequently for DVFS support.
> I think that you have to add 'struct opp_table *opp_table' instance to
> 'struct devfreq'
> and then get 'opp_table' instance in the devfreq_add_device().
Sounds good. I had wanted to do that anyway, but didn't think it was
part of this series. But I can add that change to this series.
> devfreq_add_device() already get the OPP information by using
> dev_pm_opp_get_suspend_opp_freq().
> You can add following code nearby dev_pm_opp_get_suspend_opp_freq() in
> devfreq_add_device().
> - devfreq->opp_table = dev_pm_opp_get_opp_table(dev);
Will do something like that.
I'll send out an updated patch series on Monday or Tuesday. Hopefully
Viresh would have replied by then to give his opinion on whether this
is okay by him.
Thanks,
Saravana
^ permalink raw reply
* Re: [PATCH v1 2/3] OPP: Add function to look up required OPP's for a given OPP
From: Saravana Kannan @ 2019-06-22 21:41 UTC (permalink / raw)
To: cwchoi00
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki,
Android Kernel Team, Linux PM list, linux-kernel
In-Reply-To: <CAGTfZH18SQXj_2TpUf7iQPzWrZ5RP8-OCb_t6fp7qhuutGWz5A@mail.gmail.com>
On Sat, Jun 22, 2019 at 4:50 AM Chanwoo Choi <cwchoi00@gmail.com> wrote:
>
> Hi,
>
> Absolutely, I like this approach. I think that it is necessary to make
> the connection
> between frequencies of devices.
Happy to hear that.
> But, I have a question on below.
>
> 2019년 6월 22일 (토) 오전 9:35, Saravana Kannan <saravanak@google.com>님이 작성:
> >
> > Add a function that allows looking up required OPPs given a source OPP
> > table, destination OPP table and the source OPP.
> >
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > ---
> > drivers/opp/core.c | 54 ++++++++++++++++++++++++++++++++++++++++++
> > include/linux/pm_opp.h | 11 +++++++++
> > 2 files changed, 65 insertions(+)
> >
> > diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> > index 74c7bdc6f463..4f7870bffbf8 100644
> > --- a/drivers/opp/core.c
> > +++ b/drivers/opp/core.c
> > @@ -1830,6 +1830,60 @@ void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table,
> > dev_err(virt_dev, "Failed to find required device entry\n");
> > }
> >
> > +/**
> > + * dev_pm_opp_xlate_opp() - Find required OPP for src_table OPP.
> > + * @src_table: OPP table which has dst_table as one of its required OPP table.
> > + * @dst_table: Required OPP table of the src_table.
> > + * @pstate: OPP of the src_table.
> > + *
> > + * This function returns the OPP (present in @dst_table) pointed out by the
> > + * "required-opps" property of the OPP (present in @src_table).
> > + *
> > + * The callers are required to call dev_pm_opp_put() for the returned OPP after
> > + * use.
> > + *
> > + * Return: destination table OPP on success, otherwise NULL on errors.
> > + */
> > +struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
> > + struct opp_table *dst_table,
> > + struct dev_pm_opp *src_opp)
> > +{
> > + struct dev_pm_opp *opp, *dest_opp = NULL;
> > + int i;
> > +
> > + if (!src_table || !dst_table || !src_opp)
> > + return NULL;
> > +
> > + for (i = 0; i < src_table->required_opp_count; i++) {
> > + if (src_table->required_opp_tables[i]->np == dst_table->np)
> > + break;
> > + }
> > +
> > + if (unlikely(i == src_table->required_opp_count)) {
> > + pr_err("%s: Couldn't find matching OPP table (%p: %p)\n",
> > + __func__, src_table, dst_table);
> > + return NULL;
> > + }
> > +
> > + mutex_lock(&src_table->lock);
> > +
> > + list_for_each_entry(opp, &src_table->opp_list, node) {
> > + if (opp == src_opp) {
> > + dest_opp = opp->required_opps[i];
>
> Correct me if I am wrong. This patch assume that 'i' index is same on between
> [1] and [2]. But in order to guarantee this assumption, all OPP entries
> in the same opp_table have to have the same number of 'required-opps' properties
> and keep the sequence among 'required-opps' entries.
>
> [1] src_table->required_opp_tables[i]->np
> [2] opp->required_opps[I];
>
> For example, three OPP entries in the 'parent_bus_opp'
> have the different sequence of 'required-opps' and the different
> number of 'required-opps'. Is it no problem?
>
> parent_bus_opp: opp_table {
> compatible = "operating-points-v2";
>
> opp2 {
> opp-hz = /bits/ 64 <200000>;
> required-opps = <&child_bus_a_opp2>, <&child_bus_b_opp2>,
> <&child_bus_c_opp2>;
> };
>
> opp1 {
> opp-hz = /bits/ 64 <200000>;
> // change the sequence between child_bus_b_opp2 and child_bus_c_opp2
> required-opps = <&child_bus_a_opp2>, <&child_bus_c_opp2>,
> <&child_bus_b_opp2>
> };
>
> opp0 {
> opp-hz = /bits/ 64 <200000>;
> // missing 'child_bus_a_opp2'
> required-opps = <&child_bus_c_opp2>, <&child_bus_b_opp2>
> };
>
> }
>
I get your question. If I'm not mistaken the OPP framework DT parsing
code makes the assumption that the required-opps list has the phandles
in the same order for each "row" in the OPP table. It actually only
looks at the first OPP entry to figure out the list of required OPP
tables.
Technically one can write code to deal with random order of the
required-opp list, but doesn't seem like that's worth it because
there's no need to have that order all mixed up in DT. And even if
someone wants to add support for that, I don't think improving the DT
parsing to handle random order would be part of this patch series.
-Saravana
> --
> Best Regards,
> Chanwoo Choi
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>
^ permalink raw reply
* Re: [PATCH V34 10/29] hibernate: Disable when the kernel is locked down
From: Pavel Machek @ 2019-06-22 17:52 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Josh Boyer, David Howells, Matthew Garrett, rjw, linux-pm, jikos
In-Reply-To: <20190622000358.19895-11-matthewgarrett@google.com>
[-- Attachment #1: Type: text/plain, Size: 586 bytes --]
On Fri 2019-06-21 17:03:39, Matthew Garrett wrote:
> From: Josh Boyer <jwboyer@fedoraproject.org>
>
> There is currently no way to verify the resume image when returning
> from hibernate. This might compromise the signed modules trust model,
> so until we can work with signed hibernate images we disable it when the
> kernel is locked down.
I keep getting these...
IIRC suse has patches to verify the images.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v10 13/16] sched/core: uclamp: Propagate parent clamps
From: Tejun Heo @ 2019-06-22 15:07 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, Ingo Molnar, Peter Zijlstra,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan,
Alessio Balsini
In-Reply-To: <20190621084217.8167-14-patrick.bellasi@arm.com>
Hello,
On Fri, Jun 21, 2019 at 09:42:14AM +0100, Patrick Bellasi wrote:
> Since it can be interesting for userspace, e.g. system management
> software, to know exactly what the currently propagated/enforced
> configuration is, the effective clamp values are exposed to user-space
> by means of a new pair of read-only attributes
> cpu.util.{min,max}.effective.
Can we not add the effective interface file for now? I don't think
it's a bad idea but would like to think more about it. For cpuset, it
was needed because configuration was so interwoven with the effective
masks, but we don't generally do this for other min/max or weight
knobs, all of which have effective hierarchical values and I'm not
quite sure about adding .effective for all of them. It could be that
that's what we end up doing eventually but I'd like to think a bit
more about it.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v10 12/16] sched/core: uclamp: Extend CPU's cgroup controller
From: Tejun Heo @ 2019-06-22 15:03 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, Ingo Molnar, Peter Zijlstra,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan,
Alessio Balsini
In-Reply-To: <20190621084217.8167-13-patrick.bellasi@arm.com>
Hello,
Generally looks good to me. Some nitpicks.
On Fri, Jun 21, 2019 at 09:42:13AM +0100, Patrick Bellasi wrote:
> @@ -951,6 +951,12 @@ controller implements weight and absolute bandwidth limit models for
> normal scheduling policy and absolute bandwidth allocation model for
> realtime scheduling policy.
>
> +Cycles distribution is based, by default, on a temporal base and it
> +does not account for the frequency at which tasks are executed.
> +The (optional) utilization clamping support allows to enforce a minimum
> +bandwidth, which should always be provided by a CPU, and a maximum bandwidth,
> +which should never be exceeded by a CPU.
I kinda wonder whether the term bandwidth is a bit confusing because
it's also used for cpu.max/min. Would just calling it frequency be
clearer?
> +static ssize_t cpu_uclamp_min_write(struct kernfs_open_file *of,
> + char *buf, size_t nbytes,
> + loff_t off)
> +{
> + struct task_group *tg;
> + u64 min_value;
> + int ret;
> +
> + ret = uclamp_scale_from_percent(buf, &min_value);
> + if (ret)
> + return ret;
> + if (min_value > SCHED_CAPACITY_SCALE)
> + return -ERANGE;
> +
> + rcu_read_lock();
> +
> + tg = css_tg(of_css(of));
> + if (tg == &root_task_group) {
> + ret = -EINVAL;
> + goto out;
> + }
I don't think you need the above check.
> + if (tg->uclamp_req[UCLAMP_MIN].value == min_value)
> + goto out;
> + if (tg->uclamp_req[UCLAMP_MAX].value < min_value) {
> + ret = -EINVAL;
So, uclamp.max limits the maximum freq% can get and uclamp.min limits
hte maximum freq% protection can get in the subtree. Let's say
uclamp.max is 50% and uclamp.min is 100%. It means that protection is
not limited but the actual freq% is limited upto 50%, which isn't
necessarily invalid. For a simple example, a user might be saying
that they want to get whatever protection they can get from its parent
but wanna limit eventual freq at 50% and it isn't too difficult to
imagine cases where the two knobs are configured separately especially
configuration is being managed hierarchically / automatically.
tl;dr is that we don't need the above restriction and shouldn't
generally be restricting configurations when they don't need to.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v10 09/13] drivers: devfreq: events: add Exynos PPMU new events
From: Chanwoo Choi @ 2019-06-22 13:10 UTC (permalink / raw)
To: Lukasz Luba
Cc: devicetree, linux-kernel, Linux PM list, linux-samsung-soc,
linux-clk, Michael Turquette, Stephen Boyd,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Kukjin Kim,
Chanwoo Choi, Kyungmin Park, Marek Szyprowski, Sylwester Nawrocki,
MyungJoo Ham, keescook, Tony Lindgren, jroedel, treding, digetx,
Greg KH, willy.mh.wolff.ml
In-Reply-To: <20190614095309.24100-10-l.luba@partner.samsung.com>
Hi,
2019년 6월 14일 (금) 오후 6:54, Lukasz Luba <l.luba@partner.samsung.com>님이 작성:
>
> Define new performance events supported by Exynos5422 SoC counters.
> The counters are built-in in Dynamic Memory Controller and provide
> information regarding memory utilization.
>
> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
> ---
> drivers/devfreq/event/exynos-ppmu.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/devfreq/event/exynos-ppmu.c b/drivers/devfreq/event/exynos-ppmu.c
> index c2ea94957501..ce658c262c27 100644
> --- a/drivers/devfreq/event/exynos-ppmu.c
> +++ b/drivers/devfreq/event/exynos-ppmu.c
> @@ -89,6 +89,12 @@ static struct __exynos_ppmu_events {
> PPMU_EVENT(d1-cpu),
> PPMU_EVENT(d1-general),
> PPMU_EVENT(d1-rt),
> +
> + /* For Exynos5422 SoC */
> + PPMU_EVENT(dmc0_0),
> + PPMU_EVENT(dmc0_1),
> + PPMU_EVENT(dmc1_0),
> + PPMU_EVENT(dmc1_1),
> };
>
> static int exynos_ppmu_find_ppmu_id(struct devfreq_event_dev *edev)
> --
> 2.17.1
>
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
--
Best Regards,
Chanwoo Choi
^ permalink raw reply
* Re: [PATCH v1 3/3] PM / devfreq: Add required OPPs support to passive governor
From: Chanwoo Choi @ 2019-06-22 12:00 UTC (permalink / raw)
To: Saravana Kannan
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, kernel-team,
Linux PM list, linux-kernel
In-Reply-To: <20190622003449.33707-4-saravanak@google.com>
Hi,
Absolutely, I agree this approach.
But, I add some comments on below. please check them.
2019년 6월 22일 (토) 오전 9:36, Saravana Kannan <saravanak@google.com>님이 작성:
>
> Look at the required OPPs of the "parent" device to determine the OPP that
> is required from the slave device managed by the passive governor. This
> allows having mappings between a parent device and a slave device even when
> they don't have the same number of OPPs.
>
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
> drivers/devfreq/governor_passive.c | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
> index 3bc29acbd54e..bd4a98bb15b1 100644
> --- a/drivers/devfreq/governor_passive.c
> +++ b/drivers/devfreq/governor_passive.c
> @@ -21,8 +21,9 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> struct devfreq_passive_data *p_data
> = (struct devfreq_passive_data *)devfreq->data;
> struct devfreq *parent_devfreq = (struct devfreq *)p_data->parent;
> + struct opp_table *opp_table = NULL, *c_opp_table = NULL;
In this function, the base device is passive devfreq device.
So, I think that better to define the 'parent_opp_table' instead of 'opp_table'
indicating the OPP table of parent devfreq device. And better to define
just 'opp_table' instead of 'c_opp_table' indicating the passive devfreq device.
- opp_table -> parent_opp_table
- c_opp_table -> opp_table
> unsigned long child_freq = ULONG_MAX;
> - struct dev_pm_opp *opp;
> + struct dev_pm_opp *opp = NULL, *c_opp = NULL;
Ditto. I think that better to define the variables as following:
- opp -> parent_opp
- c_cpp -> opp
> int i, count, ret = 0;
>
> /*
> @@ -65,7 +66,20 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> goto out;
> }
>
> - dev_pm_opp_put(opp);
> + opp_table = dev_pm_opp_get_opp_table(parent_devfreq->dev.parent);
devfreq_passive_get_target_freq() is called frequently for DVFS support.
I think that you have to add 'struct opp_table *opp_table' instance to
'struct devfreq'
and then get 'opp_table' instance in the devfreq_add_device().
devfreq_add_device() already get the OPP information by using
dev_pm_opp_get_suspend_opp_freq().
You can add following code nearby dev_pm_opp_get_suspend_opp_freq() in
devfreq_add_device().
- devfreq->opp_table = dev_pm_opp_get_opp_table(dev);
> + if (IS_ERR_OR_NULL(opp_table)) {
> + ret = PTR_ERR(opp_table);
> + goto out;
> + }
> +
> + c_opp_table = dev_pm_opp_get_opp_table(devfreq->dev.parent);
> + if (!IS_ERR_OR_NULL(c_opp_table))
> + c_opp = dev_pm_opp_xlate_opp(opp_table, c_opp_table, opp);
> + if (c_opp) {
> + *freq = dev_pm_opp_get_freq(c_opp);
> + dev_pm_opp_put(c_opp);
> + goto out;
> + }
>
> /*
> * Get the OPP table's index of decided freqeuncy by governor
> @@ -92,6 +106,13 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> *freq = child_freq;
>
> out:
> + if (!IS_ERR_OR_NULL(opp_table))
> + dev_pm_opp_put_opp_table(opp_table);
> + if (!IS_ERR_OR_NULL(c_opp_table))
> + dev_pm_opp_put_opp_table(c_opp_table);
> + if (!IS_ERR_OR_NULL(opp))
> + dev_pm_opp_put(opp);
> +
> return ret;
> }
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Best Regards,
Chanwoo Choi
^ permalink raw reply
* Re: [PATCH v1 2/3] OPP: Add function to look up required OPP's for a given OPP
From: Chanwoo Choi @ 2019-06-22 11:49 UTC (permalink / raw)
To: Saravana Kannan
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, kernel-team,
Linux PM list, linux-kernel
In-Reply-To: <20190622003449.33707-3-saravanak@google.com>
Hi,
Absolutely, I like this approach. I think that it is necessary to make
the connection
between frequencies of devices. But, I have a question on below.
2019년 6월 22일 (토) 오전 9:35, Saravana Kannan <saravanak@google.com>님이 작성:
>
> Add a function that allows looking up required OPPs given a source OPP
> table, destination OPP table and the source OPP.
>
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
> drivers/opp/core.c | 54 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/pm_opp.h | 11 +++++++++
> 2 files changed, 65 insertions(+)
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 74c7bdc6f463..4f7870bffbf8 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1830,6 +1830,60 @@ void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table,
> dev_err(virt_dev, "Failed to find required device entry\n");
> }
>
> +/**
> + * dev_pm_opp_xlate_opp() - Find required OPP for src_table OPP.
> + * @src_table: OPP table which has dst_table as one of its required OPP table.
> + * @dst_table: Required OPP table of the src_table.
> + * @pstate: OPP of the src_table.
> + *
> + * This function returns the OPP (present in @dst_table) pointed out by the
> + * "required-opps" property of the OPP (present in @src_table).
> + *
> + * The callers are required to call dev_pm_opp_put() for the returned OPP after
> + * use.
> + *
> + * Return: destination table OPP on success, otherwise NULL on errors.
> + */
> +struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
> + struct opp_table *dst_table,
> + struct dev_pm_opp *src_opp)
> +{
> + struct dev_pm_opp *opp, *dest_opp = NULL;
> + int i;
> +
> + if (!src_table || !dst_table || !src_opp)
> + return NULL;
> +
> + for (i = 0; i < src_table->required_opp_count; i++) {
> + if (src_table->required_opp_tables[i]->np == dst_table->np)
> + break;
> + }
> +
> + if (unlikely(i == src_table->required_opp_count)) {
> + pr_err("%s: Couldn't find matching OPP table (%p: %p)\n",
> + __func__, src_table, dst_table);
> + return NULL;
> + }
> +
> + mutex_lock(&src_table->lock);
> +
> + list_for_each_entry(opp, &src_table->opp_list, node) {
> + if (opp == src_opp) {
> + dest_opp = opp->required_opps[i];
Correct me if I am wrong. This patch assume that 'i' index is same on between
[1] and [2]. But in order to guarantee this assumption, all OPP entries
in the same opp_table have to have the same number of 'required-opps' properties
and keep the sequence among 'required-opps' entries.
[1] src_table->required_opp_tables[i]->np
[2] opp->required_opps[I];
For example, three OPP entries in the 'parent_bus_opp'
have the different sequence of 'required-opps' and the different
number of 'required-opps'. Is it no problem?
parent_bus_opp: opp_table {
compatible = "operating-points-v2";
opp2 {
opp-hz = /bits/ 64 <200000>;
required-opps = <&child_bus_a_opp2>, <&child_bus_b_opp2>,
<&child_bus_c_opp2>;
};
opp1 {
opp-hz = /bits/ 64 <200000>;
// change the sequence between child_bus_b_opp2 and child_bus_c_opp2
required-opps = <&child_bus_a_opp2>, <&child_bus_c_opp2>,
<&child_bus_b_opp2>
};
opp0 {
opp-hz = /bits/ 64 <200000>;
// missing 'child_bus_a_opp2'
required-opps = <&child_bus_c_opp2>, <&child_bus_b_opp2>
};
}
> + dev_pm_opp_get(dest_opp);
> + goto unlock;
> + }
> + }
> +
> + pr_err("%s: Couldn't find matching OPP (%p: %p)\n", __func__, src_table,
> + dst_table);
> +
> +unlock:
> + mutex_unlock(&src_table->lock);
> +
> + return dest_opp;
> +}
> +
> /**
> * dev_pm_opp_xlate_performance_state() - Find required OPP's pstate for src_table.
> * @src_table: OPP table which has dst_table as one of its required OPP table.
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index b150fe97ce5a..bc5c68bdfc8d 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -134,6 +134,9 @@ 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);
> void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table, struct device *virt_dev);
> int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate);
> +struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
> + struct opp_table *dst_table,
> + struct dev_pm_opp *src_opp);
> int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
> int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
> int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
> @@ -307,6 +310,14 @@ static inline int dev_pm_opp_xlate_performance_state(struct opp_table *src_table
> return -ENOTSUPP;
> }
>
> +static inline struct dev_pm_opp *dev_pm_opp_xlate_opp(
> + struct opp_table *src_table,
> + struct opp_table *dst_table,
> + struct dev_pm_opp *src_opp)
> +{
> + return NULL;
> +}
> +
> static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
> {
> return -ENOTSUPP;
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Best Regards,
Chanwoo Choi
^ permalink raw reply
* Re: [PATCH] cpuidle/drivers/mobile: Add new governor for mobile/embedded systems
From: kbuild test robot @ 2019-06-22 11:45 UTC (permalink / raw)
To: Daniel Lezcano
Cc: kbuild-all, rafael, linux-kernel, Rafael J. Wysocki,
Thomas Gleixner, Greg Kroah-Hartman,
open list:CPU IDLE TIME MANAGEMENT FRAMEWORK
In-Reply-To: <20190620115826.4897-1-daniel.lezcano@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]
Hi Daniel,
I love your patch! Yet something to improve:
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v5.2-rc5 next-20190621]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Lezcano/cpuidle-drivers-mobile-Add-new-governor-for-mobile-embedded-systems/20190622-064303
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 70678 bytes --]
^ permalink raw reply
* Re: [PATCH] cpuidle/drivers/mobile: Add new governor for mobile/embedded systems
From: kbuild test robot @ 2019-06-22 11:11 UTC (permalink / raw)
To: Daniel Lezcano
Cc: kbuild-all, rafael, linux-kernel, Rafael J. Wysocki,
Thomas Gleixner, Greg Kroah-Hartman,
open list:CPU IDLE TIME MANAGEMENT FRAMEWORK
In-Reply-To: <20190620115826.4897-1-daniel.lezcano@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 1249 bytes --]
Hi Daniel,
I love your patch! Yet something to improve:
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v5.2-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Lezcano/cpuidle-drivers-mobile-Add-new-governor-for-mobile-embedded-systems/20190622-064303
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: mips-allmodconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=mips
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/cpuidle/governors/mobile.o: In function `mobile_select':
>> mobile.c:(.text.mobile_select+0xe0): undefined reference to `__udivdi3'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 60704 bytes --]
^ permalink raw reply
* Re: [PATCH v2] PM / devfreq: Fix kernel oops on governor module load
From: Chanwoo Choi @ 2019-06-22 10:46 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Kyungmin Park, MyungJoo Ham, Chanwoo Choi, kernel, Linux PM list,
Enric Balletbo i Serra
In-Reply-To: <20190621213949.27018-1-ezequiel@collabora.com>
Hi,
2019년 6월 22일 (토) 오전 6:42, Ezequiel Garcia <ezequiel@collabora.com>님이 작성:
>
> A bit unexpectedly (but still documented), request_module may
> return a positive value, in case of a modprobe error.
> This is currently causing issues in the devfreq framework.
>
> When a request_module exits with a positive value, we currently
> return that via ERR_PTR. However, because the value is positive,
> it's not a ERR_VALUE proper, and is therefore treated as a
> valid struct devfreq_governor pointer, leading to a kernel oops.
>
> Fix this by returning -EINVAL if request_module returns a positive
> value.
>
> Fixes: b53b0128052ff ("PM / devfreq: Fix static checker warning in try_then_request_governor")
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> Changes from v1:
> * Rework the fix as suggested by Enric and Chanwoo,
> handling the return vaue.
> ---
> drivers/devfreq/devfreq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 6b6991f0e873..258f70c1e48f 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -257,7 +257,7 @@ static struct devfreq_governor *try_then_request_governor(const char *name)
> /* Restore previous state before return */
> mutex_lock(&devfreq_list_lock);
> if (err)
> - return ERR_PTR(err);
> + return (err < 0) ? ERR_PTR(err) : ERR_PTR(-EINVAL);
>
> governor = find_devfreq_governor(name);
> }
Thanks you for fix-up.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
But, you are missing the stable mailing list. In order to apply this
fix-up patch,\
you have to send it to stable mailing list. Please send it.
--
Best Regards,
Chanwoo Choi
^ permalink raw reply
* Re: [PATCH v4 0/7] cpufreq support for Raspberry Pi
From: Stefan Wahren @ 2019-06-22 9:54 UTC (permalink / raw)
To: Stephen Boyd, Mike Turquette
Cc: Florian Fainelli, Nicolas Saenz Julienne, linux-kernel, mbrugger,
viresh.kumar, rjw, eric, bcm-kernel-feedback-list, ptesarik,
linux-rpi-kernel, ssuloev, linux-clk, linux-arm-kernel, linux-pm
In-Reply-To: <7acfd967-0a82-5429-4eed-8b802e6620f5@i2se.com>
Hi Stephen,
hi Mike,
Am 13.06.19 um 06:31 schrieb Stefan Wahren:
> Hi Florian,
> hi Stephen,
>
> Am 13.06.19 um 05:31 schrieb Florian Fainelli:
>> On 6/12/2019 11:24 AM, Nicolas Saenz Julienne wrote:
>>> Hi all,
>>> this aims at adding cpufreq support to the Raspberry Pi family of
>>> boards.
>>>
>>> The series first factors out 'pllb' from clk-bcm2385 and creates a new
>>> clk driver that operates it over RPi's firmware interface[1]. We are
>>> forced to do so as the firmware 'owns' the pll and we're not allowed to
>>> change through the register interface directly as we might race with the
>>> over-temperature and under-voltage protections provided by the firmware.
>>>
>>> Next it creates a minimal cpufreq driver that populates the CPU's opp
>>> table, and registers cpufreq-dt. Which is needed as the firmware
>>> controls the max and min frequencies available.
>>>
>>> This was tested on a RPi3b+ and RPI2b, both using multi_v7_defconfig and
>>> arm64's defconfig.
>> How do we go about merging this? Stefan, will you pick up patch 3, 6 and
>> 7 and submit them for 5.3/5.4? Viresh has already picked up patch 4.
> is it possible to let patches 1,2, 3 and 5 go via clk-tree?
>
> I would take care of 6 and 7.
>
> Stefan
are you fine with the series, since Viresh already picked up patch 4?
are you okay with my suggestion above?
Stefan
^ permalink raw reply
* Re: [PATCH 1/6] cpufreq: Use existing stub functions instead of IS_ENABLED macro
From: Rafael J. Wysocki @ 2019-06-22 9:12 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Viresh Kumar, Eduardo Valentin, Linux Kernel Mailing List,
Rafael J. Wysocki, open list:CPU FREQUENCY SCALING FRAMEWORK
In-Reply-To: <20190621132302.30414-1-daniel.lezcano@linaro.org>
On Fri, Jun 21, 2019 at 3:23 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> The functions stub already exist for the condition the IS_ENABLED
> is trying to avoid.
>
> Remove the IS_ENABLED macros as they are pointless.
AFAICS, the IS_ENABLED checks are an optimization to avoid generating
pointless code (including a branch) in case CONFIG_CPU_THERMAL is not
set.
Why do you think that it is not useful?
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 85ff958e01f1..7c72f7d3509c 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1378,8 +1378,7 @@ static int cpufreq_online(unsigned int cpu)
> if (cpufreq_driver->ready)
> cpufreq_driver->ready(policy);
>
> - if (IS_ENABLED(CONFIG_CPU_THERMAL) &&
> - cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV)
> + if (cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV)
> policy->cdev = of_cpufreq_cooling_register(policy);
>
> pr_debug("initialization complete\n");
> @@ -1469,8 +1468,7 @@ static int cpufreq_offline(unsigned int cpu)
> goto unlock;
> }
>
> - if (IS_ENABLED(CONFIG_CPU_THERMAL) &&
> - cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV) {
> + if (cpufreq_driver->flags & CPUFREQ_IS_COOLING_DEV) {
> cpufreq_cooling_unregister(policy->cdev);
> policy->cdev = NULL;
> }
> --
> 2.17.1
>
^ permalink raw reply
* Re: [IMX] [DRM]: suspend/resume support
From: Pintu Agarwal @ 2019-06-22 8:13 UTC (permalink / raw)
To: Fabio Estevam
Cc: open list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Kernelnewbies, linux-pm
In-Reply-To: <CAOMZO5CD-QQaZwNfiX6mOLAup4J8dBiqEb_V_6jz_z5jXZ5cEw@mail.gmail.com>
On Fri, Jun 21, 2019 at 9:09 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Fri, Jun 21, 2019 at 12:13 PM Pintu Agarwal <pintu.ping@gmail.com> wrote:
>
> > Okay there is some update on the 2nd part.
> > Now I am able to successfully install all imx modules after the resume
> > (no hang).
> > But, I got some errors after install finish:
> > [drm] disabling vblank on crtc 1
> > [IMX]: imx_drm_disable_vblank - called
> > [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:24:crtc-0]
> > flip_done timed out
> >
> > Also I am able to start the weston successfully.
> > But I see LCD/HDMI display is not working (only some backlight is visible).
> >
> > And, I noticed, weston also reports the following errors:
> > imx-ipuv3 2400000.ipu: DC stop timeout after 50 ms
> > [IMX]: drm_crtc_vblank_off - called
> > [IMX]: imx_drm_disable_vblank - called
> > INFO: rcu_preempt detected stalls on CPUs/tasks: { 1} (detected by 0,
> > t=6002 jiffies, g=289, c=288, q=8)
> > Task dump for CPU 1:
> > weston R running 0 306 1 0x00000000
> > [<c05282d8>] (__schedule) from [<00080193>] (0x80193)
> >
> > Do you have any clue about these errors ?
>
> Which kernel version is this?
Please let me know in which version this issue is fixed.
I will try that.
I think I saw some commit that mentions about it, but I forgot.
Anyways, I am checking again.
Thanks!
^ permalink raw reply
* Re: [PATCH] cpuidle/drivers/mobile: Add new governor for mobile/embedded systems
From: kbuild test robot @ 2019-06-22 3:52 UTC (permalink / raw)
To: Daniel Lezcano
Cc: kbuild-all, rafael, linux-kernel, Rafael J. Wysocki,
Thomas Gleixner, Greg Kroah-Hartman,
open list:CPU IDLE TIME MANAGEMENT FRAMEWORK
In-Reply-To: <20190620115826.4897-1-daniel.lezcano@linaro.org>
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on pm/linux-next]
[also build test WARNING on v5.2-rc5 next-20190621]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Lezcano/cpuidle-drivers-mobile-Add-new-governor-for-mobile-embedded-systems/20190622-064303
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
vim +502 kernel/irq/timings.c
bbba0e7c Daniel Lezcano 2019-03-28 437
e1c92149 Daniel Lezcano 2017-06-23 438 /**
e1c92149 Daniel Lezcano 2017-06-23 439 * irq_timings_next_event - Return when the next event is supposed to arrive
e1c92149 Daniel Lezcano 2017-06-23 440 *
e1c92149 Daniel Lezcano 2017-06-23 441 * During the last busy cycle, the number of interrupts is incremented
e1c92149 Daniel Lezcano 2017-06-23 442 * and stored in the irq_timings structure. This information is
e1c92149 Daniel Lezcano 2017-06-23 443 * necessary to:
e1c92149 Daniel Lezcano 2017-06-23 444 *
e1c92149 Daniel Lezcano 2017-06-23 445 * - know if the index in the table wrapped up:
e1c92149 Daniel Lezcano 2017-06-23 446 *
e1c92149 Daniel Lezcano 2017-06-23 447 * If more than the array size interrupts happened during the
e1c92149 Daniel Lezcano 2017-06-23 448 * last busy/idle cycle, the index wrapped up and we have to
e1c92149 Daniel Lezcano 2017-06-23 449 * begin with the next element in the array which is the last one
e1c92149 Daniel Lezcano 2017-06-23 450 * in the sequence, otherwise it is a the index 0.
e1c92149 Daniel Lezcano 2017-06-23 451 *
e1c92149 Daniel Lezcano 2017-06-23 452 * - have an indication of the interrupts activity on this CPU
e1c92149 Daniel Lezcano 2017-06-23 453 * (eg. irq/sec)
e1c92149 Daniel Lezcano 2017-06-23 454 *
e1c92149 Daniel Lezcano 2017-06-23 455 * The values are 'consumed' after inserting in the statistical model,
e1c92149 Daniel Lezcano 2017-06-23 456 * thus the count is reinitialized.
e1c92149 Daniel Lezcano 2017-06-23 457 *
e1c92149 Daniel Lezcano 2017-06-23 458 * The array of values **must** be browsed in the time direction, the
e1c92149 Daniel Lezcano 2017-06-23 459 * timestamp must increase between an element and the next one.
e1c92149 Daniel Lezcano 2017-06-23 460 *
e1c92149 Daniel Lezcano 2017-06-23 461 * Returns a nanosec time based estimation of the earliest interrupt,
e1c92149 Daniel Lezcano 2017-06-23 462 * U64_MAX otherwise.
e1c92149 Daniel Lezcano 2017-06-23 463 */
e1c92149 Daniel Lezcano 2017-06-23 464 u64 irq_timings_next_event(u64 now)
e1c92149 Daniel Lezcano 2017-06-23 465 {
bbba0e7c Daniel Lezcano 2019-03-28 466 struct irq_timings *irqts = this_cpu_ptr(&irq_timings);
bbba0e7c Daniel Lezcano 2019-03-28 467 struct irqt_stat *irqs;
bbba0e7c Daniel Lezcano 2019-03-28 468 struct irqt_stat __percpu *s;
bbba0e7c Daniel Lezcano 2019-03-28 469 u64 ts, next_evt = U64_MAX;
bbba0e7c Daniel Lezcano 2019-03-28 470 int i, irq = 0;
bbba0e7c Daniel Lezcano 2019-03-28 471
e1c92149 Daniel Lezcano 2017-06-23 472 /*
e1c92149 Daniel Lezcano 2017-06-23 473 * This function must be called with the local irq disabled in
e1c92149 Daniel Lezcano 2017-06-23 474 * order to prevent the timings circular buffer to be updated
e1c92149 Daniel Lezcano 2017-06-23 475 * while we are reading it.
e1c92149 Daniel Lezcano 2017-06-23 476 */
a934d4d1 Frederic Weisbecker 2017-11-06 477 lockdep_assert_irqs_disabled();
e1c92149 Daniel Lezcano 2017-06-23 478
bbba0e7c Daniel Lezcano 2019-03-28 479 if (!irqts->count)
bbba0e7c Daniel Lezcano 2019-03-28 480 return next_evt;
bbba0e7c Daniel Lezcano 2019-03-28 481
bbba0e7c Daniel Lezcano 2019-03-28 482 /*
bbba0e7c Daniel Lezcano 2019-03-28 483 * Number of elements in the circular buffer: If it happens it
bbba0e7c Daniel Lezcano 2019-03-28 484 * was flushed before, then the number of elements could be
bbba0e7c Daniel Lezcano 2019-03-28 485 * smaller than IRQ_TIMINGS_SIZE, so the count is used,
bbba0e7c Daniel Lezcano 2019-03-28 486 * otherwise the array size is used as we wrapped. The index
bbba0e7c Daniel Lezcano 2019-03-28 487 * begins from zero when we did not wrap. That could be done
bbba0e7c Daniel Lezcano 2019-03-28 488 * in a nicer way with the proper circular array structure
bbba0e7c Daniel Lezcano 2019-03-28 489 * type but with the cost of extra computation in the
bbba0e7c Daniel Lezcano 2019-03-28 490 * interrupt handler hot path. We choose efficiency.
bbba0e7c Daniel Lezcano 2019-03-28 491 *
bbba0e7c Daniel Lezcano 2019-03-28 492 * Inject measured irq/timestamp to the pattern prediction
bbba0e7c Daniel Lezcano 2019-03-28 493 * model while decrementing the counter because we consume the
bbba0e7c Daniel Lezcano 2019-03-28 494 * data from our circular buffer.
bbba0e7c Daniel Lezcano 2019-03-28 495 */
bbba0e7c Daniel Lezcano 2019-03-28 496
bbba0e7c Daniel Lezcano 2019-03-28 497 i = (irqts->count & IRQ_TIMINGS_MASK) - 1;
bbba0e7c Daniel Lezcano 2019-03-28 498 irqts->count = min(IRQ_TIMINGS_SIZE, irqts->count);
bbba0e7c Daniel Lezcano 2019-03-28 499
bbba0e7c Daniel Lezcano 2019-03-28 500 for (; irqts->count > 0; irqts->count--, i = (i + 1) & IRQ_TIMINGS_MASK) {
bbba0e7c Daniel Lezcano 2019-03-28 501 irq = irq_timing_decode(irqts->values[i], &ts);
bbba0e7c Daniel Lezcano 2019-03-28 @502 s = idr_find(&irqt_stats, irq);
bbba0e7c Daniel Lezcano 2019-03-28 503 if (s)
bbba0e7c Daniel Lezcano 2019-03-28 504 irq_timings_store(irq, this_cpu_ptr(s), ts);
bbba0e7c Daniel Lezcano 2019-03-28 505 }
bbba0e7c Daniel Lezcano 2019-03-28 506
bbba0e7c Daniel Lezcano 2019-03-28 507 /*
bbba0e7c Daniel Lezcano 2019-03-28 508 * Look in the list of interrupts' statistics, the earliest
bbba0e7c Daniel Lezcano 2019-03-28 509 * next event.
bbba0e7c Daniel Lezcano 2019-03-28 510 */
bbba0e7c Daniel Lezcano 2019-03-28 @511 idr_for_each_entry(&irqt_stats, s, i) {
bbba0e7c Daniel Lezcano 2019-03-28 512
bbba0e7c Daniel Lezcano 2019-03-28 513 irqs = this_cpu_ptr(s);
bbba0e7c Daniel Lezcano 2019-03-28 514
bbba0e7c Daniel Lezcano 2019-03-28 515 ts = __irq_timings_next_event(irqs, i, now);
bbba0e7c Daniel Lezcano 2019-03-28 516 if (ts <= now)
bbba0e7c Daniel Lezcano 2019-03-28 517 return now;
bbba0e7c Daniel Lezcano 2019-03-28 518
bbba0e7c Daniel Lezcano 2019-03-28 519 if (ts < next_evt)
bbba0e7c Daniel Lezcano 2019-03-28 520 next_evt = ts;
bbba0e7c Daniel Lezcano 2019-03-28 521 }
bbba0e7c Daniel Lezcano 2019-03-28 522
bbba0e7c Daniel Lezcano 2019-03-28 523 return next_evt;
e1c92149 Daniel Lezcano 2017-06-23 524 }
e1c92149 Daniel Lezcano 2017-06-23 525
e1c92149 Daniel Lezcano 2017-06-23 526 void irq_timings_free(int irq)
e1c92149 Daniel Lezcano 2017-06-23 527 {
e1c92149 Daniel Lezcano 2017-06-23 528 struct irqt_stat __percpu *s;
e1c92149 Daniel Lezcano 2017-06-23 529
e1c92149 Daniel Lezcano 2017-06-23 530 s = idr_find(&irqt_stats, irq);
e1c92149 Daniel Lezcano 2017-06-23 531 if (s) {
e1c92149 Daniel Lezcano 2017-06-23 532 free_percpu(s);
e1c92149 Daniel Lezcano 2017-06-23 533 idr_remove(&irqt_stats, irq);
e1c92149 Daniel Lezcano 2017-06-23 534 }
e1c92149 Daniel Lezcano 2017-06-23 535 }
e1c92149 Daniel Lezcano 2017-06-23 536
e1c92149 Daniel Lezcano 2017-06-23 537 int irq_timings_alloc(int irq)
e1c92149 Daniel Lezcano 2017-06-23 538 {
e1c92149 Daniel Lezcano 2017-06-23 539 struct irqt_stat __percpu *s;
e1c92149 Daniel Lezcano 2017-06-23 540 int id;
e1c92149 Daniel Lezcano 2017-06-23 541
e1c92149 Daniel Lezcano 2017-06-23 542 /*
e1c92149 Daniel Lezcano 2017-06-23 543 * Some platforms can have the same private interrupt per cpu,
e1c92149 Daniel Lezcano 2017-06-23 544 * so this function may be be called several times with the
e1c92149 Daniel Lezcano 2017-06-23 545 * same interrupt number. Just bail out in case the per cpu
e1c92149 Daniel Lezcano 2017-06-23 546 * stat structure is already allocated.
e1c92149 Daniel Lezcano 2017-06-23 547 */
e1c92149 Daniel Lezcano 2017-06-23 548 s = idr_find(&irqt_stats, irq);
e1c92149 Daniel Lezcano 2017-06-23 549 if (s)
e1c92149 Daniel Lezcano 2017-06-23 550 return 0;
e1c92149 Daniel Lezcano 2017-06-23 551
e1c92149 Daniel Lezcano 2017-06-23 552 s = alloc_percpu(*s);
e1c92149 Daniel Lezcano 2017-06-23 553 if (!s)
e1c92149 Daniel Lezcano 2017-06-23 554 return -ENOMEM;
e1c92149 Daniel Lezcano 2017-06-23 555
e1c92149 Daniel Lezcano 2017-06-23 556 idr_preload(GFP_KERNEL);
e1c92149 Daniel Lezcano 2017-06-23 @557 id = idr_alloc(&irqt_stats, s, irq, irq + 1, GFP_NOWAIT);
:::::: The code at line 502 was first introduced by commit
:::::: bbba0e7c5cdadb47a91edea1d5cd0caadbbb016f genirq/timings: Add array suffix computation code
:::::: TO: Daniel Lezcano <daniel.lezcano@linaro.org>
:::::: CC: Thomas Gleixner <tglx@linutronix.de>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* [PATCH v1 1/3] OPP: Allow required-opps even if the device doesn't have power-domains
From: Saravana Kannan @ 2019-06-22 0:34 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki
Cc: Saravana Kannan, kernel-team, linux-pm, linux-kernel
In-Reply-To: <20190622003449.33707-1-saravanak@google.com>
A Device-A can have a (minimum) performance requirement on another
Device-B to be able to function correctly. This performance requirement
on Device-B can also change based on the current performance level of
Device-A.
The existing required-opps feature fits well to describe this need. So,
instead of limiting required-opps to point to only PM-domain devices,
allow it to point to any device.
Signed-off-by: Saravana Kannan <saravanak@google.com>
---
drivers/opp/core.c | 2 +-
drivers/opp/of.c | 14 --------------
2 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 0e7703fe733f..74c7bdc6f463 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -710,7 +710,7 @@ static int _set_required_opps(struct device *dev,
return 0;
/* Single genpd case */
- if (!genpd_virt_devs) {
+ if (!genpd_virt_devs && required_opp_tables[0]->is_genpd) {
pstate = opp->required_opps[0]->pstate;
ret = dev_pm_genpd_set_performance_state(dev, pstate);
if (ret) {
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index c10c782d15aa..7c8336e94aff 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -195,9 +195,6 @@ static void _opp_table_alloc_required_tables(struct opp_table *opp_table,
*/
count_pd = of_count_phandle_with_args(dev->of_node, "power-domains",
"#power-domain-cells");
- if (!count_pd)
- goto put_np;
-
if (count_pd > 1) {
genpd_virt_devs = kcalloc(count, sizeof(*genpd_virt_devs),
GFP_KERNEL);
@@ -226,17 +223,6 @@ static void _opp_table_alloc_required_tables(struct opp_table *opp_table,
if (IS_ERR(required_opp_tables[i]))
goto free_required_tables;
-
- /*
- * We only support genpd's OPPs in the "required-opps" for now,
- * as we don't know how much about other cases. Error out if the
- * required OPP doesn't belong to a genpd.
- */
- if (!required_opp_tables[i]->is_genpd) {
- dev_err(dev, "required-opp doesn't belong to genpd: %pOF\n",
- required_np);
- goto free_required_tables;
- }
}
goto put_np;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v1 3/3] PM / devfreq: Add required OPPs support to passive governor
From: Saravana Kannan @ 2019-06-22 0:34 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki
Cc: Saravana Kannan, kernel-team, linux-pm, linux-kernel
In-Reply-To: <20190622003449.33707-1-saravanak@google.com>
Look at the required OPPs of the "parent" device to determine the OPP that
is required from the slave device managed by the passive governor. This
allows having mappings between a parent device and a slave device even when
they don't have the same number of OPPs.
Signed-off-by: Saravana Kannan <saravanak@google.com>
---
drivers/devfreq/governor_passive.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index 3bc29acbd54e..bd4a98bb15b1 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -21,8 +21,9 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
struct devfreq_passive_data *p_data
= (struct devfreq_passive_data *)devfreq->data;
struct devfreq *parent_devfreq = (struct devfreq *)p_data->parent;
+ struct opp_table *opp_table = NULL, *c_opp_table = NULL;
unsigned long child_freq = ULONG_MAX;
- struct dev_pm_opp *opp;
+ struct dev_pm_opp *opp = NULL, *c_opp = NULL;
int i, count, ret = 0;
/*
@@ -65,7 +66,20 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
goto out;
}
- dev_pm_opp_put(opp);
+ opp_table = dev_pm_opp_get_opp_table(parent_devfreq->dev.parent);
+ if (IS_ERR_OR_NULL(opp_table)) {
+ ret = PTR_ERR(opp_table);
+ goto out;
+ }
+
+ c_opp_table = dev_pm_opp_get_opp_table(devfreq->dev.parent);
+ if (!IS_ERR_OR_NULL(c_opp_table))
+ c_opp = dev_pm_opp_xlate_opp(opp_table, c_opp_table, opp);
+ if (c_opp) {
+ *freq = dev_pm_opp_get_freq(c_opp);
+ dev_pm_opp_put(c_opp);
+ goto out;
+ }
/*
* Get the OPP table's index of decided freqeuncy by governor
@@ -92,6 +106,13 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
*freq = child_freq;
out:
+ if (!IS_ERR_OR_NULL(opp_table))
+ dev_pm_opp_put_opp_table(opp_table);
+ if (!IS_ERR_OR_NULL(c_opp_table))
+ dev_pm_opp_put_opp_table(c_opp_table);
+ if (!IS_ERR_OR_NULL(opp))
+ dev_pm_opp_put(opp);
+
return ret;
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v1 2/3] OPP: Add function to look up required OPP's for a given OPP
From: Saravana Kannan @ 2019-06-22 0:34 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki
Cc: Saravana Kannan, kernel-team, linux-pm, linux-kernel
In-Reply-To: <20190622003449.33707-1-saravanak@google.com>
Add a function that allows looking up required OPPs given a source OPP
table, destination OPP table and the source OPP.
Signed-off-by: Saravana Kannan <saravanak@google.com>
---
drivers/opp/core.c | 54 ++++++++++++++++++++++++++++++++++++++++++
include/linux/pm_opp.h | 11 +++++++++
2 files changed, 65 insertions(+)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 74c7bdc6f463..4f7870bffbf8 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1830,6 +1830,60 @@ void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table,
dev_err(virt_dev, "Failed to find required device entry\n");
}
+/**
+ * dev_pm_opp_xlate_opp() - Find required OPP for src_table OPP.
+ * @src_table: OPP table which has dst_table as one of its required OPP table.
+ * @dst_table: Required OPP table of the src_table.
+ * @pstate: OPP of the src_table.
+ *
+ * This function returns the OPP (present in @dst_table) pointed out by the
+ * "required-opps" property of the OPP (present in @src_table).
+ *
+ * The callers are required to call dev_pm_opp_put() for the returned OPP after
+ * use.
+ *
+ * Return: destination table OPP on success, otherwise NULL on errors.
+ */
+struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
+ struct opp_table *dst_table,
+ struct dev_pm_opp *src_opp)
+{
+ struct dev_pm_opp *opp, *dest_opp = NULL;
+ int i;
+
+ if (!src_table || !dst_table || !src_opp)
+ return NULL;
+
+ for (i = 0; i < src_table->required_opp_count; i++) {
+ if (src_table->required_opp_tables[i]->np == dst_table->np)
+ break;
+ }
+
+ if (unlikely(i == src_table->required_opp_count)) {
+ pr_err("%s: Couldn't find matching OPP table (%p: %p)\n",
+ __func__, src_table, dst_table);
+ return NULL;
+ }
+
+ mutex_lock(&src_table->lock);
+
+ list_for_each_entry(opp, &src_table->opp_list, node) {
+ if (opp == src_opp) {
+ dest_opp = opp->required_opps[i];
+ dev_pm_opp_get(dest_opp);
+ goto unlock;
+ }
+ }
+
+ pr_err("%s: Couldn't find matching OPP (%p: %p)\n", __func__, src_table,
+ dst_table);
+
+unlock:
+ mutex_unlock(&src_table->lock);
+
+ return dest_opp;
+}
+
/**
* dev_pm_opp_xlate_performance_state() - Find required OPP's pstate for src_table.
* @src_table: OPP table which has dst_table as one of its required OPP table.
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index b150fe97ce5a..bc5c68bdfc8d 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -134,6 +134,9 @@ 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);
void dev_pm_opp_put_genpd_virt_dev(struct opp_table *opp_table, struct device *virt_dev);
int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate);
+struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
+ struct opp_table *dst_table,
+ struct dev_pm_opp *src_opp);
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
@@ -307,6 +310,14 @@ static inline int dev_pm_opp_xlate_performance_state(struct opp_table *src_table
return -ENOTSUPP;
}
+static inline struct dev_pm_opp *dev_pm_opp_xlate_opp(
+ struct opp_table *src_table,
+ struct opp_table *dst_table,
+ struct dev_pm_opp *src_opp)
+{
+ return NULL;
+}
+
static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
{
return -ENOTSUPP;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v1 0/3] Add required-opps support to devfreq passive gov
From: Saravana Kannan @ 2019-06-22 0:34 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki
Cc: Saravana Kannan, kernel-team, linux-pm, linux-kernel
The devfreq passive governor scales the frequency of a "child" device
based on the current frequency of a "parent" device (not parent/child in
the sense of device hierarchy). As of today, the passive governor
requires one of the following to work correctly:
1. The parent and child device have the same number of frequencies
2. The child device driver passes a mapping function to translate from
parent frequency to child frequency.
When (1) is not true, (2) is the only option right now. But often times,
all that is required is a simple mapping from parent's frequency to
child's frequency.
Since OPPs already support pointing to other "required-opps", add
support for using that to map from parent device frequency to child
device frequency. That way, every child device driver doesn't have to
implement a separate mapping function anytime (1) isn't true.
-Saravana
Saravana Kannan (3):
OPP: Allow required-opps even if the device doesn't have power-domains
OPP: Add function to look up required OPP's for a given OPP
PM / devfreq: Add required OPPs support to passive governor
drivers/devfreq/governor_passive.c | 25 +++++++++++--
drivers/opp/core.c | 56 +++++++++++++++++++++++++++++-
drivers/opp/of.c | 14 --------
include/linux/pm_opp.h | 11 ++++++
4 files changed, 89 insertions(+), 17 deletions(-)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* [PATCH V34 10/29] hibernate: Disable when the kernel is locked down
From: Matthew Garrett @ 2019-06-22 0:03 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, Josh Boyer,
David Howells, Matthew Garrett, rjw, pavel, linux-pm
In-Reply-To: <20190622000358.19895-1-matthewgarrett@google.com>
From: Josh Boyer <jwboyer@fedoraproject.org>
There is currently no way to verify the resume image when returning
from hibernate. This might compromise the signed modules trust model,
so until we can work with signed hibernate images we disable it when the
kernel is locked down.
Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Cc: rjw@rjwysocki.net
Cc: pavel@ucw.cz
cc: linux-pm@vger.kernel.org
---
include/linux/security.h | 1 +
kernel/power/hibernate.c | 3 ++-
security/lockdown/lockdown.c | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 00a31ab2e5ba..a051f21a1144 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -85,6 +85,7 @@ enum lockdown_reason {
LOCKDOWN_MODULE_SIGNATURE,
LOCKDOWN_DEV_MEM,
LOCKDOWN_KEXEC,
+ LOCKDOWN_HIBERNATION,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index abef759de7c8..3a9cb2d3da4a 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -32,6 +32,7 @@
#include <linux/ctype.h>
#include <linux/genhd.h>
#include <linux/ktime.h>
+#include <linux/security.h>
#include <trace/events/power.h>
#include "power.h"
@@ -70,7 +71,7 @@ static const struct platform_hibernation_ops *hibernation_ops;
bool hibernation_available(void)
{
- return (nohibernate == 0);
+ return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
}
/**
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 08fcd8116db3..ce5b3da9bd09 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -21,6 +21,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
[LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
[LOCKDOWN_KEXEC] = "kexec of unsigned images",
+ [LOCKDOWN_HIBERNATION] = "hibernation",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v2] PM / devfreq: Fix kernel oops on governor module load
From: Ezequiel Garcia @ 2019-06-21 21:39 UTC (permalink / raw)
To: Kyungmin Park, MyungJoo Ham, Chanwoo Choi
Cc: kernel, linux-pm, Enric Balletbo i Serra, Ezequiel Garcia
A bit unexpectedly (but still documented), request_module may
return a positive value, in case of a modprobe error.
This is currently causing issues in the devfreq framework.
When a request_module exits with a positive value, we currently
return that via ERR_PTR. However, because the value is positive,
it's not a ERR_VALUE proper, and is therefore treated as a
valid struct devfreq_governor pointer, leading to a kernel oops.
Fix this by returning -EINVAL if request_module returns a positive
value.
Fixes: b53b0128052ff ("PM / devfreq: Fix static checker warning in try_then_request_governor")
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
Changes from v1:
* Rework the fix as suggested by Enric and Chanwoo,
handling the return vaue.
---
drivers/devfreq/devfreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 6b6991f0e873..258f70c1e48f 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -257,7 +257,7 @@ static struct devfreq_governor *try_then_request_governor(const char *name)
/* Restore previous state before return */
mutex_lock(&devfreq_list_lock);
if (err)
- return ERR_PTR(err);
+ return (err < 0) ? ERR_PTR(err) : ERR_PTR(-EINVAL);
governor = find_devfreq_governor(name);
}
--
2.20.1
^ permalink raw reply related
* Re: [IMX] [DRM]: suspend/resume support
From: Fabio Estevam @ 2019-06-21 15:40 UTC (permalink / raw)
To: Pintu Agarwal
Cc: open list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Kernelnewbies, linux-pm
In-Reply-To: <CAOuPNLhstoCjxijrnKNmV1iKWjAXvSZ38Z13tfd5bvGbYSqPAA@mail.gmail.com>
On Fri, Jun 21, 2019 at 12:13 PM Pintu Agarwal <pintu.ping@gmail.com> wrote:
> Okay there is some update on the 2nd part.
> Now I am able to successfully install all imx modules after the resume
> (no hang).
> But, I got some errors after install finish:
> [drm] disabling vblank on crtc 1
> [IMX]: imx_drm_disable_vblank - called
> [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:24:crtc-0]
> flip_done timed out
>
> Also I am able to start the weston successfully.
> But I see LCD/HDMI display is not working (only some backlight is visible).
>
> And, I noticed, weston also reports the following errors:
> imx-ipuv3 2400000.ipu: DC stop timeout after 50 ms
> [IMX]: drm_crtc_vblank_off - called
> [IMX]: imx_drm_disable_vblank - called
> INFO: rcu_preempt detected stalls on CPUs/tasks: { 1} (detected by 0,
> t=6002 jiffies, g=289, c=288, q=8)
> Task dump for CPU 1:
> weston R running 0 306 1 0x00000000
> [<c05282d8>] (__schedule) from [<00080193>] (0x80193)
>
> Do you have any clue about these errors ?
Which kernel version is this?
^ permalink raw reply
* Re: [IMX] [DRM]: suspend/resume support
From: Pintu Agarwal @ 2019-06-21 15:12 UTC (permalink / raw)
To: Fabio Estevam
Cc: open list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Kernelnewbies, linux-pm
In-Reply-To: <CAOuPNLjrAU_C_TUKFMs1d0eGsw=AxuG6d6FhNHtHFwVhfYZGgA@mail.gmail.com>
On Wed, Jun 19, 2019 at 8:59 PM Pintu Agarwal <pintu.ping@gmail.com> wrote:
> > > This scenario is not with suspend/resume.
> > > This hang is, when we make hdmi as a loadable module (.ko) and trying
> > > to install it after resume.
> > > In this case, suspend/resume will not come into picture. Not sure why
> > > it still hangs.
> > > Do you have any clue for this scenario?
> >
> > I haven't tried this one.
> >
Okay there is some update on the 2nd part.
Now I am able to successfully install all imx modules after the resume
(no hang).
But, I got some errors after install finish:
[drm] disabling vblank on crtc 1
[IMX]: imx_drm_disable_vblank - called
[drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:24:crtc-0]
flip_done timed out
Also I am able to start the weston successfully.
But I see LCD/HDMI display is not working (only some backlight is visible).
And, I noticed, weston also reports the following errors:
imx-ipuv3 2400000.ipu: DC stop timeout after 50 ms
[IMX]: drm_crtc_vblank_off - called
[IMX]: imx_drm_disable_vblank - called
INFO: rcu_preempt detected stalls on CPUs/tasks: { 1} (detected by 0,
t=6002 jiffies, g=289, c=288, q=8)
Task dump for CPU 1:
weston R running 0 306 1 0x00000000
[<c05282d8>] (__schedule) from [<00080193>] (0x80193)
Do you have any clue about these errors ?
Thanks,
Pintu
^ 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