* Re: [PATCH] PCI / PM: Do not clear state_saved for devices that remain suspended
From: Rafael J. Wysocki @ 2018-05-23 8:11 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Rafael J. Wysocki, Linux PCI, Linux PM, LKML, Bjorn Helgaas,
Mika Westerberg
In-Reply-To: <20180522220149.GB22385@bhelgaas-glaptop.roam.corp.google.com>
On Wed, May 23, 2018 at 12:01 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Fri, May 18, 2018 at 10:17:42AM +0200, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> The state_saved flag should not be cleared in pci_pm_suspend() if the
>> given device is going to remain suspended, or the device's config
>> space will not be restored properly during the subsequent resume.
>>
>> Namely, if the device is going to stay in suspend, both the late
>> and noirq callbacks return early for it, so if its state_saved flag
>> is cleared in pci_pm_suspend(), it will remain unset throughout the
>> remaining part of suspend and resume and pci_restore_state() called
>> for the device going forward will return without doing anything.
>>
>> For this reason, change pci_pm_suspend() to only clear state_saved
>> if the given device is not going to remain suspended. [This is
>> analogous to what commit ae860a19f37c (PCI / PM: Do not clear
>> state_saved in pci_pm_freeze() when smart suspend is set) did for
>> hibernation.]
>>
>> Fixes: c4b65157aeef (PCI / PM: Take SMART_SUSPEND driver flag into account)
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>
> I assume you'll take this one, too.
Yes, I will, thank you!
^ permalink raw reply
* Re: [PATCH V3] powercap/drivers/idle_injection: Add an idle injection framework
From: Daniel Lezcano @ 2018-05-23 8:00 UTC (permalink / raw)
To: Viresh Kumar
Cc: rjw, edubezval, kevin.wangtao, leo.yan, vincent.guittot,
linux-kernel, javi.merino, rui.zhang, linux-pm, daniel.thompson
In-Reply-To: <20180523054154.fvhnwgvq34ihivdj@vireshk-i7>
On 23/05/2018 07:41, Viresh Kumar wrote:
> On 22-05-18, 15:42, Daniel Lezcano wrote:
>> On 21/05/2018 12:32, Viresh Kumar wrote:
>>> On 18-05-18, 16:50, Daniel Lezcano wrote:
>>>> Initially, the cpu_cooling device for ARM was changed by adding a new
>>>> policy inserting idle cycles. The intel_powerclamp driver does a
>>>> similar action.
>>>>
>>>> Instead of implementing idle injections privately in the cpu_cooling
>>>> device, move the idle injection code in a dedicated framework and give
>>>> the opportunity to other frameworks to make use of it.
>>>
>>> I thought you agreed to move above in the comments section ?
>>
>> This is what I did. I just kept the relevant log here.
>
> The fact that you are stating that you tried to update the cooling
> device earlier looked like a bit of version history to me, not what
> this patch is doing.
>
> But its okay if you really want that to be preserved in git history :)
>
>>>> +static void idle_injection_fn(unsigned int cpu)
>>>> +{
>>>> + struct idle_injection_device *ii_dev;
>>>> + struct idle_injection_thread *iit;
>>>> + int run_duration_ms, idle_duration_ms;
>>>> +
>>>> + ii_dev = per_cpu(idle_injection_device, cpu);
>>>> +
>>>> + iit = per_cpu_ptr(&idle_injection_thread, cpu);
>>>> +
>>>> + /*
>>>> + * Boolean used by the smpboot mainloop and used as a flip-flop
>>>> + * in this function
>>>> + */
>>>> + iit->should_run = 0;
>>>> +
>>>> + atomic_inc(&ii_dev->count);
>>>> +
>>>> + idle_duration_ms = atomic_read(&ii_dev->idle_duration_ms);
>>>> +
>>>> + play_idle(idle_duration_ms);
>>>> +
>>>> + /*
>>>> + * The last CPU waking up is in charge of setting the timer. If
>>>> + * the CPU is hotplugged, the timer will move to another CPU
>>>> + * (which may not belong to the same cluster) but that is not a
>>>> + * problem as the timer will be set again by another CPU
>>>> + * belonging to the cluster. This mechanism is self adaptive.
>>>> + */
>>>> + if (!atomic_dec_and_test(&ii_dev->count))
>>>> + return;
>>>> +
>>>> + run_duration_ms = atomic_read(&ii_dev->run_duration_ms);
>>>
>>> This reads as if it is okay to have run_duration_ms set as 0, so we
>>> run idle loop only once. Which is fine, but why do you mandate this to
>>> be non-zero in idle_injection_start() ?
>>
>> It does not make sense to run this function with a run duration set to
>> zero because we will immediately go to idle again after exiting idle. So
>> the action is exiting. In this context we can't accept to start
>> injecting idle cycles.
>
> Right and that's why I said "Which is fine" in my comment above. My
> question was more on why we error out in idle_injection_start() if
> run_duration_ms is 0.
>
> Just for my understanding, is it a valid usecase where we want to run
> the idle loop only once ? i.e. set idle_duration_ms to a non-zero
> value but run_duration_ms to 0 ? In that case we shouldn't check for
> zero run_duration_ms in idle_injection_start().
Yes, that could be a valid use case if we want to synchronously inject
idle cycles without period.
IOW, call play_idle() on a set of cpus at the same time. And the caller
of start is the one with the control of the period.
If you want this usecase, we need to implement more things:
- single user of the framework: as soon as we register, no-one else can
use the idle injection
- blocking stop, we wait for all the kthreads to join a barrier before
returning to the caller
- blocking start, we wait for all the kthreads to end injecting the
idle cycle
>>>> + if (!run_duration_ms)
>>>> + return;
>>>> +
>>>> + hrtimer_start(&ii_dev->timer, ms_to_ktime(run_duration_ms),
>>>> + HRTIMER_MODE_REL_PINNED);
>>>> +}
>>>> +
>>>> +/**
>>>> + * idle_injection_set_duration - idle and run duration helper
>>>> + * @run_duration_ms: an unsigned int giving the running time in milliseconds
>>>> + * @idle_duration_ms: an unsigned int giving the idle time in milliseconds
>>>> + */
>>>> +void idle_injection_set_duration(struct idle_injection_device *ii_dev,
>>>> + unsigned int run_duration_ms,
>>>> + unsigned int idle_duration_ms)
>>>> +{
>>>> + atomic_set(&ii_dev->run_duration_ms, run_duration_ms);
>>>> + atomic_set(&ii_dev->idle_duration_ms, idle_duration_ms);
>>>
>>> You check for valid values of these in idle_injection_start() but not
>>> here, why ?
>>
>> By checking against a zero values in the start function is a way to make
>> sure we are not starting the idle injection with uninitialized values
>> and by setting the duration to zero is a way to stop the idle injection.
>
> Why do we need two ways of stopping the idle injection thread ? Why
> isn't just calling idle_injection_stop() the right thing to do in that
> case ?
How do we prevent the last kthread in the idle_injection_fn to set the
timer ?
>>>> +}
>>>> +
>>>> +/**
>>>> + * idle_injection_get_duration - idle and run duration helper
>>>> + * @run_duration_ms: a pointer to an unsigned int to store the running time
>>>> + * @idle_duration_ms: a pointer to an unsigned int to store the idle time
>>>> + */
>>>> +void idle_injection_get_duration(struct idle_injection_device *ii_dev,
>>>> + unsigned int *run_duration_ms,
>>>> + unsigned int *idle_duration_ms)
>>>> +{
>>>> + *run_duration_ms = atomic_read(&ii_dev->run_duration_ms);
>>>> + *idle_duration_ms = atomic_read(&ii_dev->idle_duration_ms);
>>>> +}
>>>> +
>>>> +/**
>>>> + * idle_injection_start - starts the idle injections
>>>> + * @ii_dev: a pointer to an idle_injection_device structure
>>>> + *
>>>> + * The function starts the idle injection cycles by first waking up
>>>> + * all the tasks the ii_dev is attached to and let them handle the
>>>> + * idle-run periods.
>>>> + *
>>>> + * Return: -EINVAL if the idle or the running durations are not set.
>>>> + */
>>>> +int idle_injection_start(struct idle_injection_device *ii_dev)
>>>> +{
>>>> + if (!atomic_read(&ii_dev->idle_duration_ms))
>>>> + return -EINVAL;
>>>> +
>>>> + if (!atomic_read(&ii_dev->run_duration_ms))
>>>> + return -EINVAL;
>>>> +
>
> So according to above comments from me, I am saying that this
> particular test isn't really required as we may want to run idle loop
> only once.
Note that will be the same than calling play_idle() synchronously.
>>>> + pr_debug("Starting injecting idle cycles on CPUs '%*pbl'\n",
>>>> + cpumask_pr_args(ii_dev->cpumask));
>>>> +
>>>> + idle_injection_wakeup(ii_dev);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +/**
>>>> + * idle_injection_stop - stops the idle injections
>>>> + * @ii_dev: a pointer to an idle injection_device structure
>>>> + *
>>>> + * The function stops the idle injection by canceling the timer in
>>>> + * charge of waking up the tasks to inject idle and unset the idle and
>>>> + * running durations.
>>>> + */
>>>> +void idle_injection_stop(struct idle_injection_device *ii_dev)
>>>> +{
>>>> + pr_debug("Stopping injecting idle cycles on CPUs '%*pbl'\n",
>>>> + cpumask_pr_args(ii_dev->cpumask));
>>>> +
>>>> + hrtimer_cancel(&ii_dev->timer);
>>>
>>> How are we sure that idle_injection_fn() isn't running at this point
>>> and it would start the timer cancelled here again ?
>>
>> Nothing will ensure that. We will have an extra idle injection in this
>> case. We can invert the set_duration(0,0) and the timer cancellation to
>> reduce to reduce the window.
>
> That's what I thought and so its racy. If someone calls
> idle_injection_unregister(), then we call this routine and then free
> the data structures while they are still getting used by the thread :(
Yes, we need to make the framework single-user, a refcount should be
enough. However, register() returns a pointer and the caller of
unregister must have this pointer. If it is the case, then register and
unregister code collaborate, if the one calling unregister cuts the
branch of the user of the idle_injection then we have braindead code.
We can handle this case by adding locks or we can have a single-user of
the framework without lock. We don't expect a lot of idle injection
users (I see only two right now and they are mutually exclusive), so
having lockless code is ok for me.
>>>> +
>>>> + idle_injection_set_duration(ii_dev, 0, 0);
>>>
>>> And why exactly this this required ? Why shouldn't we allow this
>>> sequence to work:
>>>
>>> idle_injection_set_duration()
>>> idle_injection_start()
>>> idle_injection_stop()
>>> idle_injection_start()
>>> idle_injection_stop()
>>> idle_injection_start()
>>> idle_injection_stop()
>>
>> Sorry, I don't get it.
>>
>> Who will decide to start() and stop() ?
>
> Some confusion here about the usecase then. How do you see this stuff
> getting used and how users (cooling-driver ?) will use it ?
The cooling device computes the duration and sets it every time
set_cur_state is called.
when the mitigation begins (prev state = 0 and cur state > 0), it starts
the threads.
When the mitigation ends (prev_state > 0 and cur_state = 0), it calls stop()
In between, set_duration is called via set_cur_state to vary the
mitigation effect.
--
<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 19/33] thermal: db8500: use match_string() helper
From: Yisheng Xie @ 2018-05-23 7:47 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linux Kernel Mailing List, Zhang Rui, Eduardo Valentin, Linux PM
In-Reply-To: <CAHp75VfqEdoDZydwFmQaLiUN8cVtH3MbFHf3qixU8xUHXMwt1g@mail.gmail.com>
Hi Andy,
On 2018/5/22 6:00, Andy Shevchenko wrote:
> On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie <xieyisheng1@huawei.com> wrote:
>> match_string() returns the index of an array for a matching string,
>> which can be used intead of open coded variant.
>
>> + i = match_string((const char **)trip_point->cdev_name,
>
> Casting looks ugly. You need to constify the variable itself.
When I tried to const cdev_name like:
+++ b/include/linux/platform_data/db8500_thermal.h
@@ -27,7 +27,7 @@
struct db8500_trip_point {
unsigned long temp;
enum thermal_trip_type type;
- char cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH];
+ char const cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH]; // const char cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH] will also the same
};
The compiler will also warning:
drivers/thermal/db8500_thermal.c: In function ‘db8500_thermal_match_cdev’:
drivers/thermal/db8500_thermal.c:53:2: warning: passing argument 1 of ‘match_string’ from incompatible pointer type [enabled by default]
i = match_string(trip_point->cdev_name, COOLING_DEV_MAX, cdev->type);
^
In file included from include/linux/bitmap.h:9:0,
from include/linux/cpumask.h:12,
from include/linux/rcupdate.h:44,
from include/linux/radix-tree.h:28,
from include/linux/idr.h:15,
from include/linux/kernfs.h:14,
from include/linux/sysfs.h:16,
from kernel/include/linux/kobject.h:20,
from kernel/include/linux/of.h:17,
from include/linux/cpu_cooling.h:27,
from drivers/thermal/db8500_thermal.c:20:
include/linux/string.h:184:5: note: expected ‘const char * const*’ but argument is of type ‘const char (*)[20]’
Any idea?
Thanks
Yisheng
>
>> + COOLING_DEV_MAX, cdev->type);
>>
>> - return -ENODEV;
>> + return (i < 0) ? -ENODEV : 0;
>
> I would rather go with
>
> if (ret < 0)
> return -ENODEV;
>
> return 0;
>
^ permalink raw reply
* [PATCH v3 13/13] soc: rockchip: power-domain: add power domain support for px30
From: Elaine Zhang @ 2018-05-23 6:53 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao, Finley Xiao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
From: Finley Xiao <finley.xiao@rock-chips.com>
This driver is modified to support PX30 SoC.
Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
drivers/soc/rockchip/pm_domains.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c
index 90dcd5e21ae6..d0c5615132e3 100644
--- a/drivers/soc/rockchip/pm_domains.c
+++ b/drivers/soc/rockchip/pm_domains.c
@@ -18,6 +18,7 @@
#include <linux/clk.h>
#include <linux/regmap.h>
#include <linux/mfd/syscon.h>
+#include <dt-bindings/power/px30-power.h>
#include <dt-bindings/power/rk3036-power.h>
#include <dt-bindings/power/rk3128-power.h>
#include <dt-bindings/power/rk3228-power.h>
@@ -114,6 +115,9 @@ struct rockchip_pmu {
.active_wakeup = wakeup, \
}
+#define DOMAIN_PX30(pwr, status, req, wakeup) \
+ DOMAIN_M(pwr, status, req, (req) + 16, req, wakeup)
+
#define DOMAIN_RK3288(pwr, status, req, wakeup) \
DOMAIN(pwr, status, req, req, (req) + 16, wakeup)
@@ -712,6 +716,17 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
return error;
}
+static const struct rockchip_domain_info px30_pm_domains[] = {
+ [PX30_PD_USB] = DOMAIN_PX30(5, 5, 10, false),
+ [PX30_PD_SDCARD] = DOMAIN_PX30(8, 8, 9, false),
+ [PX30_PD_GMAC] = DOMAIN_PX30(10, 10, 6, false),
+ [PX30_PD_MMC_NAND] = DOMAIN_PX30(11, 11, 5, false),
+ [PX30_PD_VPU] = DOMAIN_PX30(12, 12, 14, false),
+ [PX30_PD_VO] = DOMAIN_PX30(13, 13, 7, false),
+ [PX30_PD_VI] = DOMAIN_PX30(14, 14, 8, false),
+ [PX30_PD_GPU] = DOMAIN_PX30(15, 15, 2, false),
+};
+
static const struct rockchip_domain_info rk3036_pm_domains[] = {
[RK3036_PD_MSCH] = DOMAIN_RK3036(14, 23, 30, true),
[RK3036_PD_CORE] = DOMAIN_RK3036(13, 17, 24, false),
@@ -811,6 +826,17 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
[RK3399_PD_SDIOAUDIO] = DOMAIN_RK3399(31, 31, 29, true),
};
+static const struct rockchip_pmu_info px30_pmu = {
+ .pwr_offset = 0x18,
+ .status_offset = 0x20,
+ .req_offset = 0x64,
+ .idle_offset = 0x6c,
+ .ack_offset = 0x6c,
+
+ .num_domains = ARRAY_SIZE(px30_pm_domains),
+ .domain_info = px30_pm_domains,
+};
+
static const struct rockchip_pmu_info rk3036_pmu = {
.req_offset = 0x148,
.idle_offset = 0x14c,
@@ -915,6 +941,10 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
static const struct of_device_id rockchip_pm_domain_dt_match[] = {
{
+ .compatible = "rockchip,px30-power-controller",
+ .data = (void *)&px30_pmu,
+ },
+ {
.compatible = "rockchip,rk3036-power-controller",
.data = (void *)&rk3036_pmu,
},
--
1.9.1
^ permalink raw reply related
* [PATCH v3 12/13] dt-bindings: add binding for px30 power domains
From: Elaine Zhang @ 2018-05-23 6:52 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao, Finley Xiao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
From: Finley Xiao <finley.xiao@rock-chips.com>
Add binding documentation for the power domains
found on Rockchip PX30 SoCs.
Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
Documentation/devicetree/bindings/soc/rockchip/power_domain.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
index affe36dcfa17..5d49d0a2ff29 100644
--- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
+++ b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
@@ -5,6 +5,7 @@ powered up/down by software based on different application scenes to save power.
Required properties for power domain controller:
- compatible: Should be one of the following.
+ "rockchip,px30-power-controller" - for PX30 SoCs.
"rockchip,rk3036-power-controller" - for RK3036 SoCs.
"rockchip,rk3128-power-controller" - for RK3128 SoCs.
"rockchip,rk3228-power-controller" - for RK3228 SoCs.
@@ -20,6 +21,7 @@ Required properties for power domain controller:
Required properties for power domain sub nodes:
- reg: index of the power domain, should use macros in:
+ "include/dt-bindings/power/px30-power.h" - for PX30 type power domain.
"include/dt-bindings/power/rk3036-power.h" - for RK3036 type power domain.
"include/dt-bindings/power/rk3128-power.h" - for RK3128 type power domain.
"include/dt-bindings/power/rk3228-power.h" - for RK3228 type power domain.
@@ -99,6 +101,7 @@ Node of a device using power domains must have a power-domains property,
containing a phandle to the power device node and an index specifying which
power domain to use.
The index should use macros in:
+ "include/dt-bindings/power/px30-power.h" - for px30 type power domain.
"include/dt-bindings/power/rk3036-power.h" - for rk3036 type power domain.
"include/dt-bindings/power/rk3128-power.h" - for rk3128 type power domain.
"include/dt-bindings/power/rk3128-power.h" - for rk3228 type power domain.
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 2/2] typec: tcpm: Provide fwnode pointer as part of psy_cfg
From: Heikki Krogerus @ 2018-05-23 6:52 UTC (permalink / raw)
To: Adam Thomson
Cc: Guenter Roeck, Greg Kroah-Hartman, Sebastian Reichel, linux-usb,
linux-pm, linux-kernel, support.opensource
In-Reply-To: <449707040719c8845cf9fa6bc779bae25fca54a5.1527000797.git.Adam.Thomson.Opensource@diasemi.com>
On Tue, May 22, 2018 at 04:16:24PM +0100, Adam Thomson wrote:
> For supply registration, provide fwnode pointer of the port device,
> via the power_supply_config structure, to allow other psy drivers
> to add us as a supplier. At present this only applies to DT
> based platforms using the 'power-supplies' DT property, but in the
> future should also work for ACPI platforms when the relevant support
> is added to the power_supply core.
>
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 72996cc..0ccd2ce 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -14,6 +14,7 @@
> #include <linux/mutex.h>
> #include <linux/power_supply.h>
> #include <linux/proc_fs.h>
> +#include <linux/property.h>
> #include <linux/sched/clock.h>
> #include <linux/seq_file.h>
> #include <linux/slab.h>
> @@ -4500,6 +4501,7 @@ static int devm_tcpm_psy_register(struct tcpm_port *port)
> char *psy_name;
>
> psy_cfg.drv_data = port;
> + psy_cfg.fwnode = dev_fwnode(port->dev);
> psy_name = devm_kzalloc(port->dev, psy_name_len, GFP_KERNEL);
> if (!psy_name)
> return -ENOMEM;
Thanks,
--
heikki
^ permalink raw reply
* [PATCH v3 11/13] dt-bindings: power: add PX30 SoCs header for power-domain
From: Elaine Zhang @ 2018-05-23 6:52 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao, Finley Xiao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
From: Finley Xiao <finley.xiao@rock-chips.com>
According to a description from TRM, add all the power domains.
Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
include/dt-bindings/power/px30-power.h | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
create mode 100644 include/dt-bindings/power/px30-power.h
diff --git a/include/dt-bindings/power/px30-power.h b/include/dt-bindings/power/px30-power.h
new file mode 100644
index 000000000000..30917a99ad20
--- /dev/null
+++ b/include/dt-bindings/power/px30-power.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DT_BINDINGS_POWER_PX30_POWER_H__
+#define __DT_BINDINGS_POWER_PX30_POWER_H__
+
+/* VD_CORE */
+#define PX30_PD_A35_0 0
+#define PX30_PD_A35_1 1
+#define PX30_PD_A35_2 2
+#define PX30_PD_A35_3 3
+#define PX30_PD_SCU 4
+
+/* VD_LOGIC */
+#define PX30_PD_USB 5
+#define PX30_PD_DDR 6
+#define PX30_PD_SDCARD 7
+#define PX30_PD_CRYPTO 8
+#define PX30_PD_GMAC 9
+#define PX30_PD_MMC_NAND 10
+#define PX30_PD_VPU 11
+#define PX30_PD_VO 12
+#define PX30_PD_VI 13
+#define PX30_PD_GPU 14
+
+/* VD_PMU */
+#define PX30_PD_PMU 15
+
+#endif
--
1.9.1
^ permalink raw reply related
* [PATCH v3 10/13] soc: rockchip: power-domain: add power domain support for rk3228
From: Elaine Zhang @ 2018-05-23 6:52 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
This driver is modified to support RK3228 SoC.
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
drivers/soc/rockchip/pm_domains.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c
index 99a2dd8a7801..90dcd5e21ae6 100644
--- a/drivers/soc/rockchip/pm_domains.c
+++ b/drivers/soc/rockchip/pm_domains.c
@@ -20,6 +20,7 @@
#include <linux/mfd/syscon.h>
#include <dt-bindings/power/rk3036-power.h>
#include <dt-bindings/power/rk3128-power.h>
+#include <dt-bindings/power/rk3228-power.h>
#include <dt-bindings/power/rk3288-power.h>
#include <dt-bindings/power/rk3328-power.h>
#include <dt-bindings/power/rk3366-power.h>
@@ -729,6 +730,20 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
[RK3128_PD_GPU] = DOMAIN_RK3288(1, 1, 3, false),
};
+static const struct rockchip_domain_info rk3228_pm_domains[] = {
+ [RK3228_PD_CORE] = DOMAIN_RK3036(0, 0, 16, true),
+ [RK3228_PD_MSCH] = DOMAIN_RK3036(1, 1, 17, true),
+ [RK3228_PD_BUS] = DOMAIN_RK3036(2, 2, 18, true),
+ [RK3228_PD_SYS] = DOMAIN_RK3036(3, 3, 19, true),
+ [RK3228_PD_VIO] = DOMAIN_RK3036(4, 4, 20, false),
+ [RK3228_PD_VOP] = DOMAIN_RK3036(5, 5, 21, false),
+ [RK3228_PD_VPU] = DOMAIN_RK3036(6, 6, 22, false),
+ [RK3228_PD_RKVDEC] = DOMAIN_RK3036(7, 7, 23, false),
+ [RK3228_PD_GPU] = DOMAIN_RK3036(8, 8, 24, false),
+ [RK3228_PD_PERI] = DOMAIN_RK3036(9, 9, 25, true),
+ [RK3228_PD_GMAC] = DOMAIN_RK3036(10, 10, 26, false),
+};
+
static const struct rockchip_domain_info rk3288_pm_domains[] = {
[RK3288_PD_VIO] = DOMAIN_RK3288(7, 7, 4, false),
[RK3288_PD_HEVC] = DOMAIN_RK3288(14, 10, 9, false),
@@ -816,6 +831,15 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
.domain_info = rk3128_pm_domains,
};
+static const struct rockchip_pmu_info rk3228_pmu = {
+ .req_offset = 0x40c,
+ .idle_offset = 0x488,
+ .ack_offset = 0x488,
+
+ .num_domains = ARRAY_SIZE(rk3228_pm_domains),
+ .domain_info = rk3228_pm_domains,
+};
+
static const struct rockchip_pmu_info rk3288_pmu = {
.pwr_offset = 0x08,
.status_offset = 0x0c,
@@ -899,6 +923,10 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
.data = (void *)&rk3128_pmu,
},
{
+ .compatible = "rockchip,rk3228-power-controller",
+ .data = (void *)&rk3228_pmu,
+ },
+ {
.compatible = "rockchip,rk3288-power-controller",
.data = (void *)&rk3288_pmu,
},
--
1.9.1
^ permalink raw reply related
* [PATCH v3 09/13] dt-bindings: add binding for rk3228 power domains
From: Elaine Zhang @ 2018-05-23 6:51 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
Add binding documentation for the power domains
found on Rockchip RK3228 SoCs.
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/soc/rockchip/power_domain.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
index 9a3f5fd36a80..affe36dcfa17 100644
--- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
+++ b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
@@ -7,6 +7,7 @@ Required properties for power domain controller:
- compatible: Should be one of the following.
"rockchip,rk3036-power-controller" - for RK3036 SoCs.
"rockchip,rk3128-power-controller" - for RK3128 SoCs.
+ "rockchip,rk3228-power-controller" - for RK3228 SoCs.
"rockchip,rk3288-power-controller" - for RK3288 SoCs.
"rockchip,rk3328-power-controller" - for RK3328 SoCs.
"rockchip,rk3366-power-controller" - for RK3366 SoCs.
@@ -21,6 +22,7 @@ Required properties for power domain sub nodes:
- reg: index of the power domain, should use macros in:
"include/dt-bindings/power/rk3036-power.h" - for RK3036 type power domain.
"include/dt-bindings/power/rk3128-power.h" - for RK3128 type power domain.
+ "include/dt-bindings/power/rk3228-power.h" - for RK3228 type power domain.
"include/dt-bindings/power/rk3288-power.h" - for RK3288 type power domain.
"include/dt-bindings/power/rk3328-power.h" - for RK3328 type power domain.
"include/dt-bindings/power/rk3366-power.h" - for RK3366 type power domain.
@@ -99,6 +101,7 @@ power domain to use.
The index should use macros in:
"include/dt-bindings/power/rk3036-power.h" - for rk3036 type power domain.
"include/dt-bindings/power/rk3128-power.h" - for rk3128 type power domain.
+ "include/dt-bindings/power/rk3128-power.h" - for rk3228 type power domain.
"include/dt-bindings/power/rk3288-power.h" - for rk3288 type power domain.
"include/dt-bindings/power/rk3328-power.h" - for rk3328 type power domain.
"include/dt-bindings/power/rk3366-power.h" - for rk3366 type power domain.
--
1.9.1
^ permalink raw reply related
* [PATCH v3 08/13] dt-bindings: power: add RK3228 SoCs header for power-domain
From: Elaine Zhang @ 2018-05-23 6:51 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
According to a description from TRM, add all the power domains.
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
include/dt-bindings/power/rk3228-power.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 include/dt-bindings/power/rk3228-power.h
diff --git a/include/dt-bindings/power/rk3228-power.h b/include/dt-bindings/power/rk3228-power.h
new file mode 100644
index 000000000000..6a8dc1bf76ce
--- /dev/null
+++ b/include/dt-bindings/power/rk3228-power.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DT_BINDINGS_POWER_RK3228_POWER_H__
+#define __DT_BINDINGS_POWER_RK3228_POWER_H__
+
+/**
+ * RK3228 idle id Summary.
+ */
+
+#define RK3228_PD_CORE 0
+#define RK3228_PD_MSCH 1
+#define RK3228_PD_BUS 2
+#define RK3228_PD_SYS 3
+#define RK3228_PD_VIO 4
+#define RK3228_PD_VOP 5
+#define RK3228_PD_VPU 6
+#define RK3228_PD_RKVDEC 7
+#define RK3228_PD_GPU 8
+#define RK3228_PD_PERI 9
+#define RK3228_PD_GMAC 10
+
+#endif
--
1.9.1
^ permalink raw reply related
* [PATCH v3 07/13] soc: rockchip: power-domain: add power domain support for rk3128
From: Elaine Zhang @ 2018-05-23 6:51 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
This driver is modified to support RK3128 SoC.
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
drivers/soc/rockchip/pm_domains.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c
index 01d4ba26a054..99a2dd8a7801 100644
--- a/drivers/soc/rockchip/pm_domains.c
+++ b/drivers/soc/rockchip/pm_domains.c
@@ -19,6 +19,7 @@
#include <linux/regmap.h>
#include <linux/mfd/syscon.h>
#include <dt-bindings/power/rk3036-power.h>
+#include <dt-bindings/power/rk3128-power.h>
#include <dt-bindings/power/rk3288-power.h>
#include <dt-bindings/power/rk3328-power.h>
#include <dt-bindings/power/rk3366-power.h>
@@ -720,6 +721,14 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
[RK3036_PD_SYS] = DOMAIN_RK3036(8, 22, 29, false),
};
+static const struct rockchip_domain_info rk3128_pm_domains[] = {
+ [RK3128_PD_CORE] = DOMAIN_RK3288(0, 0, 4, false),
+ [RK3128_PD_MSCH] = DOMAIN_RK3288(-1, -1, 6, true),
+ [RK3128_PD_VIO] = DOMAIN_RK3288(3, 3, 2, false),
+ [RK3128_PD_VIDEO] = DOMAIN_RK3288(2, 2, 1, false),
+ [RK3128_PD_GPU] = DOMAIN_RK3288(1, 1, 3, false),
+};
+
static const struct rockchip_domain_info rk3288_pm_domains[] = {
[RK3288_PD_VIO] = DOMAIN_RK3288(7, 7, 4, false),
[RK3288_PD_HEVC] = DOMAIN_RK3288(14, 10, 9, false),
@@ -796,6 +805,17 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
.domain_info = rk3036_pm_domains,
};
+static const struct rockchip_pmu_info rk3128_pmu = {
+ .pwr_offset = 0x04,
+ .status_offset = 0x08,
+ .req_offset = 0x0c,
+ .idle_offset = 0x10,
+ .ack_offset = 0x10,
+
+ .num_domains = ARRAY_SIZE(rk3128_pm_domains),
+ .domain_info = rk3128_pm_domains,
+};
+
static const struct rockchip_pmu_info rk3288_pmu = {
.pwr_offset = 0x08,
.status_offset = 0x0c,
@@ -875,6 +895,10 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
.data = (void *)&rk3036_pmu,
},
{
+ .compatible = "rockchip,rk3128-power-controller",
+ .data = (void *)&rk3128_pmu,
+ },
+ {
.compatible = "rockchip,rk3288-power-controller",
.data = (void *)&rk3288_pmu,
},
--
1.9.1
^ permalink raw reply related
* [PATCH v3 06/13] dt-bindings: add binding for rk3128 power domains
From: Elaine Zhang @ 2018-05-23 6:50 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
Add binding documentation for the power domains
found on Rockchip RK3128 SoCs.
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/soc/rockchip/power_domain.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
index 79924ee9ae86..9a3f5fd36a80 100644
--- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
+++ b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
@@ -6,6 +6,7 @@ powered up/down by software based on different application scenes to save power.
Required properties for power domain controller:
- compatible: Should be one of the following.
"rockchip,rk3036-power-controller" - for RK3036 SoCs.
+ "rockchip,rk3128-power-controller" - for RK3128 SoCs.
"rockchip,rk3288-power-controller" - for RK3288 SoCs.
"rockchip,rk3328-power-controller" - for RK3328 SoCs.
"rockchip,rk3366-power-controller" - for RK3366 SoCs.
@@ -19,6 +20,7 @@ Required properties for power domain controller:
Required properties for power domain sub nodes:
- reg: index of the power domain, should use macros in:
"include/dt-bindings/power/rk3036-power.h" - for RK3036 type power domain.
+ "include/dt-bindings/power/rk3128-power.h" - for RK3128 type power domain.
"include/dt-bindings/power/rk3288-power.h" - for RK3288 type power domain.
"include/dt-bindings/power/rk3328-power.h" - for RK3328 type power domain.
"include/dt-bindings/power/rk3366-power.h" - for RK3366 type power domain.
@@ -96,6 +98,7 @@ containing a phandle to the power device node and an index specifying which
power domain to use.
The index should use macros in:
"include/dt-bindings/power/rk3036-power.h" - for rk3036 type power domain.
+ "include/dt-bindings/power/rk3128-power.h" - for rk3128 type power domain.
"include/dt-bindings/power/rk3288-power.h" - for rk3288 type power domain.
"include/dt-bindings/power/rk3328-power.h" - for rk3328 type power domain.
"include/dt-bindings/power/rk3366-power.h" - for rk3366 type power domain.
--
1.9.1
^ permalink raw reply related
* [PATCH v3 05/13] dt-bindings: power: add RK3128 SoCs header for power-domain
From: Elaine Zhang @ 2018-05-23 6:50 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
According to a description from TRM, add all the power domains.
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
include/dt-bindings/power/rk3128-power.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 include/dt-bindings/power/rk3128-power.h
diff --git a/include/dt-bindings/power/rk3128-power.h b/include/dt-bindings/power/rk3128-power.h
new file mode 100644
index 000000000000..c051dc3108db
--- /dev/null
+++ b/include/dt-bindings/power/rk3128-power.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DT_BINDINGS_POWER_RK3128_POWER_H__
+#define __DT_BINDINGS_POWER_RK3128_POWER_H__
+
+/* VD_CORE */
+#define RK3128_PD_CORE 0
+
+/* VD_LOGIC */
+#define RK3128_PD_VIO 1
+#define RK3128_PD_VIDEO 2
+#define RK3128_PD_GPU 3
+#define RK3128_PD_MSCH 4
+
+#endif
--
1.9.1
^ permalink raw reply related
* [PATCH v3 04/13] soc: rockchip: power-domain: Fix wrong value when power up pd
From: Elaine Zhang @ 2018-05-23 6:48 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao, Finley Xiao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
From: Finley Xiao <finley.xiao@rock-chips.com>
Solve the pd could only ever turn off but never turn them on again,
If the pd registers have the writemask bits.
Fix up the code error for commit:
commit 79bb17ce8edb3141339b5882e372d0ec7346217c
Author: Elaine Zhang <zhangqing@rock-chips.com>
Date: Fri Dec 23 11:47:52 2016 +0800
soc: rockchip: power-domain: Support domain control in hiword-registers
New Rockchips SoCs may have their power-domain control in registers
using a writemask-based access scheme (upper 16bit being the write
mask). So add a DOMAIN_M type and handle this case accordingly.
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
drivers/soc/rockchip/pm_domains.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c
index ebd7c41898c0..01d4ba26a054 100644
--- a/drivers/soc/rockchip/pm_domains.c
+++ b/drivers/soc/rockchip/pm_domains.c
@@ -264,7 +264,7 @@ static void rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
return;
else if (pd->info->pwr_w_mask)
regmap_write(pmu->regmap, pmu->info->pwr_offset,
- on ? pd->info->pwr_mask :
+ on ? pd->info->pwr_w_mask :
(pd->info->pwr_mask | pd->info->pwr_w_mask));
else
regmap_update_bits(pmu->regmap, pmu->info->pwr_offset,
--
1.9.1
^ permalink raw reply related
* [PATCH v3 03/13] Soc: rockchip: power-domain: add power domain support for rk3036
From: Elaine Zhang @ 2018-05-23 6:48 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
From: Caesar Wang <wxt@rock-chips.com>
This driver is modified to support RK3036 SoC.
Signed-off-by: Caesar Wang <wxt@rock-chips.com>
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
drivers/soc/rockchip/pm_domains.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c
index 53efc386b1ad..ebd7c41898c0 100644
--- a/drivers/soc/rockchip/pm_domains.c
+++ b/drivers/soc/rockchip/pm_domains.c
@@ -18,6 +18,7 @@
#include <linux/clk.h>
#include <linux/regmap.h>
#include <linux/mfd/syscon.h>
+#include <dt-bindings/power/rk3036-power.h>
#include <dt-bindings/power/rk3288-power.h>
#include <dt-bindings/power/rk3328-power.h>
#include <dt-bindings/power/rk3366-power.h>
@@ -102,6 +103,14 @@ struct rockchip_pmu {
.ack_mask = (ack >= 0) ? BIT(ack) : 0, \
.active_wakeup = wakeup, \
}
+#define DOMAIN_RK3036(req, ack, idle, wakeup) \
+{ \
+ .req_mask = (req >= 0) ? BIT(req) : 0, \
+ .req_w_mask = (req >= 0) ? BIT(req + 16) : 0, \
+ .ack_mask = (ack >= 0) ? BIT(ack) : 0, \
+ .idle_mask = (idle >= 0) ? BIT(idle) : 0, \
+ .active_wakeup = wakeup, \
+}
#define DOMAIN_RK3288(pwr, status, req, wakeup) \
DOMAIN(pwr, status, req, req, (req) + 16, wakeup)
@@ -701,6 +710,16 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
return error;
}
+static const struct rockchip_domain_info rk3036_pm_domains[] = {
+ [RK3036_PD_MSCH] = DOMAIN_RK3036(14, 23, 30, true),
+ [RK3036_PD_CORE] = DOMAIN_RK3036(13, 17, 24, false),
+ [RK3036_PD_PERI] = DOMAIN_RK3036(12, 18, 25, false),
+ [RK3036_PD_VIO] = DOMAIN_RK3036(11, 19, 26, false),
+ [RK3036_PD_VPU] = DOMAIN_RK3036(10, 20, 27, false),
+ [RK3036_PD_GPU] = DOMAIN_RK3036(9, 21, 28, false),
+ [RK3036_PD_SYS] = DOMAIN_RK3036(8, 22, 29, false),
+};
+
static const struct rockchip_domain_info rk3288_pm_domains[] = {
[RK3288_PD_VIO] = DOMAIN_RK3288(7, 7, 4, false),
[RK3288_PD_HEVC] = DOMAIN_RK3288(14, 10, 9, false),
@@ -768,6 +787,15 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
[RK3399_PD_SDIOAUDIO] = DOMAIN_RK3399(31, 31, 29, true),
};
+static const struct rockchip_pmu_info rk3036_pmu = {
+ .req_offset = 0x148,
+ .idle_offset = 0x14c,
+ .ack_offset = 0x14c,
+
+ .num_domains = ARRAY_SIZE(rk3036_pm_domains),
+ .domain_info = rk3036_pm_domains,
+};
+
static const struct rockchip_pmu_info rk3288_pmu = {
.pwr_offset = 0x08,
.status_offset = 0x0c,
@@ -843,6 +871,10 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
static const struct of_device_id rockchip_pm_domain_dt_match[] = {
{
+ .compatible = "rockchip,rk3036-power-controller",
+ .data = (void *)&rk3036_pmu,
+ },
+ {
.compatible = "rockchip,rk3288-power-controller",
.data = (void *)&rk3288_pmu,
},
--
1.9.1
^ permalink raw reply related
* [PATCH v3 02/13] dt-bindings: add binding for rk3036 power domains
From: Elaine Zhang @ 2018-05-23 6:48 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
From: Caesar Wang <wxt@rock-chips.com>
Add binding documentation for the power domains
found on Rockchip RK3036 SoCs.
Signed-off-by: Caesar Wang <wxt@rock-chips.com>
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/soc/rockchip/power_domain.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
index 301d2a9bc1b8..79924ee9ae86 100644
--- a/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
+++ b/Documentation/devicetree/bindings/soc/rockchip/power_domain.txt
@@ -5,6 +5,7 @@ powered up/down by software based on different application scenes to save power.
Required properties for power domain controller:
- compatible: Should be one of the following.
+ "rockchip,rk3036-power-controller" - for RK3036 SoCs.
"rockchip,rk3288-power-controller" - for RK3288 SoCs.
"rockchip,rk3328-power-controller" - for RK3328 SoCs.
"rockchip,rk3366-power-controller" - for RK3366 SoCs.
@@ -17,6 +18,7 @@ Required properties for power domain controller:
Required properties for power domain sub nodes:
- reg: index of the power domain, should use macros in:
+ "include/dt-bindings/power/rk3036-power.h" - for RK3036 type power domain.
"include/dt-bindings/power/rk3288-power.h" - for RK3288 type power domain.
"include/dt-bindings/power/rk3328-power.h" - for RK3328 type power domain.
"include/dt-bindings/power/rk3366-power.h" - for RK3366 type power domain.
@@ -93,6 +95,7 @@ Node of a device using power domains must have a power-domains property,
containing a phandle to the power device node and an index specifying which
power domain to use.
The index should use macros in:
+ "include/dt-bindings/power/rk3036-power.h" - for rk3036 type power domain.
"include/dt-bindings/power/rk3288-power.h" - for rk3288 type power domain.
"include/dt-bindings/power/rk3328-power.h" - for rk3328 type power domain.
"include/dt-bindings/power/rk3366-power.h" - for rk3366 type power domain.
--
1.9.1
^ permalink raw reply related
* [PATCH v3 01/13] dt-bindings: power: add RK3036 SoCs header for power-domain
From: Elaine Zhang @ 2018-05-23 6:48 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao,
Elaine Zhang
In-Reply-To: <1527058129-10260-1-git-send-email-zhangqing@rock-chips.com>
From: Caesar Wang <wxt@rock-chips.com>
According to a description from TRM, add all the power domains.
Signed-off-by: Caesar Wang <wxt@rock-chips.com>
Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
include/dt-bindings/power/rk3036-power.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
create mode 100644 include/dt-bindings/power/rk3036-power.h
diff --git a/include/dt-bindings/power/rk3036-power.h b/include/dt-bindings/power/rk3036-power.h
new file mode 100644
index 000000000000..0bc6b5d5075e
--- /dev/null
+++ b/include/dt-bindings/power/rk3036-power.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DT_BINDINGS_POWER_RK3036_POWER_H__
+#define __DT_BINDINGS_POWER_RK3036_POWER_H__
+
+#define RK3036_PD_MSCH 0
+#define RK3036_PD_CORE 1
+#define RK3036_PD_PERI 2
+#define RK3036_PD_VIO 3
+#define RK3036_PD_VPU 4
+#define RK3036_PD_GPU 5
+#define RK3036_PD_SYS 6
+
+#endif
--
1.9.1
^ permalink raw reply related
* [PATCH v3 00/13] add power domain support for Rockchip Socs
From: Elaine Zhang @ 2018-05-23 6:48 UTC (permalink / raw)
To: heiko, robh+dt, mark.rutland
Cc: devicetree, rjw, khilman, ulf.hansson, linux-pm, linux-arm-kernel,
linux-rockchip, linux-kernel, wxt, xxx, xf, huangtao,
Elaine Zhang
add power domain support for RK3036/RK3128/RK3228/PX30 Soc.
fix up the wrong value when set power domain up.
Change in V2:
Fix up the commit message description and Assign author.
Change in V3:
[PATCH 01/13]: The Copyright description use SPDX tag instead.
[PATCH 05/13]: The Copyright description use SPDX tag instead.
[PATCH 08/13]: The Copyright description use SPDX tag instead.
[PATCH 11/13]: The Copyright description use SPDX tag instead.
Caesar Wang (3):
dt-bindings: power: add RK3036 SoCs header for power-domain
dt-bindings: add binding for rk3036 power domains
Soc: rockchip: power-domain: add power domain support for rk3036
Elaine Zhang (6):
dt-bindings: power: add RK3128 SoCs header for power-domain
dt-bindings: add binding for rk3128 power domains
soc: rockchip: power-domain: add power domain support for rk3128
dt-bindings: power: add RK3228 SoCs header for power-domain
dt-bindings: add binding for rk3228 power domains
soc: rockchip: power-domain: add power domain support for rk3228
Finley Xiao (4):
soc: rockchip: power-domain: Fix wrong value when power up pd
dt-bindings: power: add PX30 SoCs header for power-domain
dt-bindings: add binding for px30 power domains
soc: rockchip: power-domain: add power domain support for px30
.../bindings/soc/rockchip/power_domain.txt | 12 +++
drivers/soc/rockchip/pm_domains.c | 116 ++++++++++++++++++++-
include/dt-bindings/power/px30-power.h | 27 +++++
include/dt-bindings/power/rk3036-power.h | 13 +++
include/dt-bindings/power/rk3128-power.h | 14 +++
include/dt-bindings/power/rk3228-power.h | 21 ++++
6 files changed, 202 insertions(+), 1 deletion(-)
create mode 100644 include/dt-bindings/power/px30-power.h
create mode 100644 include/dt-bindings/power/rk3036-power.h
create mode 100644 include/dt-bindings/power/rk3128-power.h
create mode 100644 include/dt-bindings/power/rk3228-power.h
--
1.9.1
^ permalink raw reply
* Re: [PATCH 1/2] power: supply: Add fwnode pointer to power_supply_config struct
From: Heikki Krogerus @ 2018-05-23 6:48 UTC (permalink / raw)
To: Adam Thomson
Cc: Guenter Roeck, Greg Kroah-Hartman, Sebastian Reichel, linux-usb,
linux-pm, linux-kernel, support.opensource
In-Reply-To: <c398d1fcc81017b9cc694a00d2c1f655874dfeac.1527000797.git.Adam.Thomson.Opensource@diasemi.com>
On Tue, May 22, 2018 at 04:16:23PM +0100, Adam Thomson wrote:
> To allow users of the power supply framework to be hw description
> agnostic, this commit adds the ability to pass a fwnode pointer,
> via the power_supply_config structure, to the initialisation code
> of the core, instead of explicitly specifying of_ndoe. If that
> fwnode pointer is provided then it will automatically resolve down
> to of_node on platforms which support it, otherwise it will be NULL.
>
> In the future, when ACPI support is added, this can be modified to
> accommodate ACPI without the need to change calling code which
> already provides the fwnode handle in this manner.
>
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Thanks Adam! FWIW:
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/power/supply/power_supply_core.c | 4 +++-
> include/linux/power_supply.h | 2 ++
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index ecd68c2..f57ab0a 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -19,6 +19,7 @@
> #include <linux/err.h>
> #include <linux/of.h>
> #include <linux/power_supply.h>
> +#include <linux/property.h>
> #include <linux/thermal.h>
> #include "power_supply.h"
>
> @@ -874,7 +875,8 @@ static void psy_unregister_cooler(struct power_supply *psy)
> psy->desc = desc;
> if (cfg) {
> psy->drv_data = cfg->drv_data;
> - psy->of_node = cfg->of_node;
> + psy->of_node =
> + cfg->fwnode ? to_of_node(cfg->fwnode) : cfg->of_node;
> psy->supplied_to = cfg->supplied_to;
> psy->num_supplicants = cfg->num_supplicants;
> }
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 0c9a572..b21c4bd9 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -199,6 +199,8 @@ enum power_supply_notifier_events {
> /* Run-time specific power supply configuration */
> struct power_supply_config {
> struct device_node *of_node;
> + struct fwnode_handle *fwnode;
> +
> /* Driver private data */
> void *drv_data;
Thanks,
--
heikki
^ permalink raw reply
* Re: [PATCH RFC] schedutil: Address the r/w ordering race in kthread
From: Juri Lelli @ 2018-05-23 6:47 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Joel Fernandes (Google), Rafael J . Wysocki,
Peter Zijlstra, Ingo Molnar, Patrick Bellasi, Luca Abeni,
Todd Kjos, claudio, kernel-team, linux-pm
In-Reply-To: <20180522235028.80564-1-joel@joelfernandes.org>
Hi Joel,
On 22/05/18 16:50, Joel Fernandes (Google) wrote:
> Currently there is a race in schedutil code for slow-switch single-CPU
> systems. Fix it by enforcing ordering the write to work_in_progress to
> happen before the read of next_freq.
>
> Kthread Sched update
>
> sugov_work() sugov_update_single()
>
> lock();
> // The CPU is free to rearrange below
> // two in any order, so it may clear
> // the flag first and then read next
> // freq. Lets assume it does.
> work_in_progress = false
>
> if (work_in_progress)
> return;
>
> sg_policy->next_freq = 0;
> freq = sg_policy->next_freq;
> sg_policy->next_freq = real-freq;
> unlock();
>
> Reported-by: Viresh Kumar <viresh.kumar@linaro.org>
> CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: Patrick Bellasi <patrick.bellasi@arm.com>
> CC: Juri Lelli <juri.lelli@redhat.com>
> Cc: Luca Abeni <luca.abeni@santannapisa.it>
> CC: Todd Kjos <tkjos@google.com>
> CC: claudio@evidence.eu.com
> CC: kernel-team@android.com
> CC: linux-pm@vger.kernel.org
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> I split this into separate patch, because this race can also happen in
> mainline.
>
> kernel/sched/cpufreq_schedutil.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index 5c482ec38610..ce7749da7a44 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -401,6 +401,13 @@ static void sugov_work(struct kthread_work *work)
> */
> raw_spin_lock_irqsave(&sg_policy->update_lock, flags);
> freq = sg_policy->next_freq;
> +
> + /*
> + * sugov_update_single can access work_in_progress without update_lock,
> + * make sure next_freq is read before work_in_progress is set.
s/set/reset/
> + */
> + smp_mb();
> +
Also, doesn't this need a corresponding barrier (I guess in
sugov_should_update_freq)? That being a wmb and this a rmb?
Best,
- Juri
^ permalink raw reply
* Re: [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Ulf Hansson @ 2018-05-23 6:12 UTC (permalink / raw)
To: Rajendra Nayak, Jon Hunter
Cc: Geert Uytterhoeven, Linux PM, Greg Kroah-Hartman, Kevin Hilman,
Rafael J . Wysocki, Linux Kernel Mailing List, Todor Tomov,
Viresh Kumar, linux-tegra, Vincent Guittot, Linux ARM
In-Reply-To: <51f7de26-579a-8b9e-4e79-f4eee923ab38@codeaurora.org>
Rajendra, Jon,
On 23 May 2018 at 06:51, Rajendra Nayak <rnayak@codeaurora.org> wrote:
>
>
> On 05/23/2018 02:25 AM, Jon Hunter wrote:
>>
>> On 22/05/18 15:47, Ulf Hansson wrote:
>>> [...]
>>>
>>>>>
>>>>> +/**
>>>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>>>> + * @dev: Device to attach.
>>>>> + * @index: The index of the PM domain.
>>>>> + *
>>>>> + * Parse device's OF node to find a PM domain specifier at the provided @index.
>>>>> + * If such is found, allocates a new device and attaches it to retrieved
>>>>> + * pm_domain ops.
>>>>> + *
>>>>> + * Returns the allocated device if successfully attached PM domain, NULL when
>>>>> + * the device don't need a PM domain or have a single PM domain, else PTR_ERR()
>>>>> + * in case of failures. Note that if a power-domain exists for the device, but
>>>>> + * cannot be found or turned on, then return PTR_ERR(-EPROBE_DEFER) to ensure
>>>>> + * that the device is not probed and to re-try again later.
>>>>> + */
>>>>> +struct device *genpd_dev_pm_attach_by_id(struct device *dev,
>>>>> + unsigned int index)
>>>>> +{
>>>>> + struct device *genpd_dev;
>>>>> + int num_domains;
>>>>> + int ret;
>>>>> +
>>>>> + if (!dev->of_node)
>>>>> + return NULL;
>>>>> +
>>>>> + /* Deal only with devices using multiple PM domains. */
>>>>> + num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
>>>>> + "#power-domain-cells");
>>>>> + if (num_domains < 2 || index >= num_domains)
>>>>> + return NULL;
>>>>> +
>>>>> + /* Allocate and register device on the genpd bus. */
>>>>> + genpd_dev = kzalloc(sizeof(*genpd_dev), GFP_KERNEL);
>>>>> + if (!genpd_dev)
>>>>> + return ERR_PTR(-ENOMEM);
>>>>> +
>>>>> + dev_set_name(genpd_dev, "genpd:%u:%s", index, dev_name(dev));
>>>>> + genpd_dev->bus = &genpd_bus_type;
>>>>> + genpd_dev->release = genpd_release_dev;
>>>>> +
>>>>> + ret = device_register(genpd_dev);
>>>>> + if (ret) {
>>>>> + kfree(genpd_dev);
>>>>> + return ERR_PTR(ret);
>>>>> + }
>>>>> +
>>>>> + /* Try to attach the device to the PM domain at the specified index. */
>>>>> + ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index);
>>>>> + if (ret < 1) {
>>>>> + device_unregister(genpd_dev);
>>>>> + return ret ? ERR_PTR(ret) : NULL;
>>>>> + }
>>>>> +
>>>>> + pm_runtime_set_active(genpd_dev);
>>>>> + pm_runtime_enable(genpd_dev);
>>>>> +
>>>>> + return genpd_dev;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_by_id);
>>>>
>>>> Thanks for sending this. Believe it or not this has still been on my to-do list
>>>> and so we definitely need a solution for Tegra.
>>>>
>>>> Looking at the above it appears that additional power-domains exposed as devices
>>>> to the client device. So I assume that this means that the drivers for devices
>>>> with multiple power-domains will need to call RPM APIs for each of these
>>>> additional power-domains. Is that correct?
>>>
>>> They can, but should not!
>>>
>>> Instead, the driver shall use device_link_add() and device_link_del(),
>>> dynamically, depending on what PM domain that their original device
>>> needs for the current running use case.
>>>
>>> In that way, they keep existing runtime PM deployment, operating on
>>> its original device.
>>
>> OK, sounds good. Any reason why the linking cannot be handled by the above API? Is there a use-case where you would not want it linked?
>
> I am guessing the linking is what would give the driver the ability to decide which subset of powerdomains it actually wants to control
> at any point using runtime PM. If we have cases wherein the driver would want to turn on/off _all_ its associated powerdomains _always_
> then a default linking of all would help.
First, I think we need to decide on *where* the linking should be
done, not at both places, as that would just mess up synchronization
of who is responsible for calling the device_link_del() at detach.
Second, It would in principle be fine to call device_link_add() and
device_link_del() as a part of the attach/detach APIs. However, there
is a downside to such solution, which would be that the driver then
needs call the detach API, just to do device_link_del(). Of course
then it would also needs to call the attach API later if/when needed.
Doing this adds unnecessary overhead - comparing to just let the
driver call device_link_add|del() when needed. On the upside, yes, it
would put less burden on the drivers as it then only needs to care
about using one set of functions.
Which solution do you prefer?
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v1] cpufreq: tegra20: Fix imbalanced clock enable count
From: Viresh Kumar @ 2018-05-23 5:58 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Rafael J. Wysocki, Thierry Reding, Jonathan Hunter, linux-tegra,
linux-pm, linux-kernel
In-Reply-To: <20180522211420.2006-1-digetx@gmail.com>
On 23-05-18, 00:14, Dmitry Osipenko wrote:
> Tegra20-cpufreq driver missed enabling the CPU clocks. This results in a
> clock-enable refcount disbalance on PLL_P <-> PLL_X reparent, causing
> PLL_X to get disabled while it shouldn't. Fix this by enabling the clocks
> on the driver probe.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>
> CPUFreq maintainers,
>
> Please take into account that this patch is made on top of my recent
> series of patches [0] "Clean up Tegra20 cpufreq driver" that was fully
> reviewed, but seems not applied yet. Let me know if you prefer to re-spin
> the [0], including this patch into the series.
>
> [0] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=45321
This is already picked by Rafael and is sitting in pm/bleeding-edge
branch. Should get merged into linux-next in a day or two.
> drivers/cpufreq/tegra20-cpufreq.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
> index 05f57dcd5215..ca5229265b60 100644
> --- a/drivers/cpufreq/tegra20-cpufreq.c
> +++ b/drivers/cpufreq/tegra20-cpufreq.c
> @@ -176,6 +176,14 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
> goto put_pll_x;
> }
>
> + err = clk_prepare_enable(cpufreq->pll_x_clk);
> + if (err)
> + goto put_pll_p;
> +
> + err = clk_prepare_enable(cpufreq->pll_p_clk);
> + if (err)
> + goto disable_pll_x;
> +
> cpufreq->dev = &pdev->dev;
> cpufreq->driver.get = cpufreq_generic_get;
> cpufreq->driver.attr = cpufreq_generic_attr;
> @@ -192,12 +200,16 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
>
> err = cpufreq_register_driver(&cpufreq->driver);
> if (err)
> - goto put_pll_p;
> + goto disable_pll_p;
>
> platform_set_drvdata(pdev, cpufreq);
>
> return 0;
>
> +disable_pll_p:
> + clk_disable_unprepare(cpufreq->pll_p_clk);
> +disable_pll_x:
> + clk_disable_unprepare(cpufreq->pll_x_clk);
> put_pll_p:
> clk_put(cpufreq->pll_p_clk);
> put_pll_x:
> @@ -214,6 +226,8 @@ static int tegra20_cpufreq_remove(struct platform_device *pdev)
>
> cpufreq_unregister_driver(&cpufreq->driver);
>
> + clk_disable_unprepare(cpufreq->pll_p_clk);
> + clk_disable_unprepare(cpufreq->pll_x_clk);
> clk_put(cpufreq->pll_p_clk);
> clk_put(cpufreq->pll_x_clk);
> clk_put(cpufreq->cpu_clk);
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH v2 2/2] cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver
From: Viresh Kumar @ 2018-05-23 5:54 UTC (permalink / raw)
To: Taniya Das
Cc: skannan, Rafael J. Wysocki, linux-kernel, linux-pm, Stephen Boyd,
robh, Rajendra Nayak, Amit Nischal, devicetree, amit.kucheria
In-Reply-To: <bf5b09d3-4d97-5357-3fb2-926227bb7229@codeaurora.org>
On 22-05-18, 16:13, Taniya Das wrote:
> On 5/22/2018 12:36 AM, skannan@codeaurora.org wrote:
> > On 2018-05-21 02:01, Viresh Kumar wrote:
> > > On 19-05-18, 23:04, Taniya Das wrote:
> > > > + /* Make sure the write goes through before proceeding */
> > > > + mb();
> > >
> > > Btw what happens right after this is done ? Are we guaranteed that the
> > > frequency is updated in the hardware after this ? What about enabling
> > > fast-switch for your platform ? Look at drivers/cpufreq/scmi-cpufreq.c
> > > to see how that is done.
> >
> > Yeah, I don't think this is needed really.
> >
>
> Just want to make sure it doesn't really sit in the write buffer before
> return.
As per Saravana you will be dropping it now, right ?
> > > > + index = readl_relaxed(c->perf_base);
> > > > + index = min(index, LUT_MAX_ENTRIES - 1);
> > >
> > > Will the hardware ever read a value over 39 here ?
> >
> > The register could be initialized to whatever before the kernel is
> > brought up. Don't want to depend on it being correct to avoid out of
> > bounds access that could leak data.
Fair enough.
> > > > +static int qcom_read_lut(struct platform_device *pdev,
> > > > + struct cpufreq_qcom *c)
> > > > +{
> > > > + struct device *dev = &pdev->dev;
> > > > + u32 data, src, lval, i, core_count, prev_cc;
> > > > +
> > > > + c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
> > > > + sizeof(*c->table), GFP_KERNEL);
> > > > + if (!c->table)
> > > > + return -ENOMEM;
> > > > +
> > > > + for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> > > > + data = readl_relaxed(c->lut_base + i * LUT_ROW_SIZE);
> > > > + src = ((data & GENMASK(31, 30)) >> 30);
> > > > + lval = (data & GENMASK(7, 0));
> > > > + core_count = CORE_COUNT_VAL(data);
> > > > +
> > > > + if (!src)
> > > > + c->table[i].frequency = INIT_RATE / 1000;
> > > > + else
> > > > + c->table[i].frequency = XO_RATE * lval / 1000;
> > > > +
> > > > + c->table[i].driver_data = c->table[i].frequency;
> > >
> > > Why do you need to use driver_data here? Why can't you simple use
> > > frequency field in the below conditional expressions ?
> > >
>
> The frequency field would be marked INVALID in case the core count does not
> match and the frequency data would be lost.
>
> > > > +
> > > > + dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
> > > > + i, c->table[i].frequency, core_count);
> > > > +
> > > > + if (core_count != c->max_cores)
> > > > + c->table[i].frequency = CPUFREQ_ENTRY_INVALID;
> > > > +
> > > > + /*
> > > > + * Two of the same frequencies with the same core counts means
> > > > + * end of table.
> > > > + */
> > > > + if (i > 0 && c->table[i - 1].driver_data ==
> > > > + c->table[i].driver_data && prev_cc == core_count) {
> > > > + struct cpufreq_frequency_table *prev = &c->table[i - 1];
> > > > +
> > > > + if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
> > >
> > > There can only be a single boost frequency at max ?
> >
> > As of now, yes. If that changes, we'll change this code later.
> >
> > > > + prev->flags = CPUFREQ_BOOST_FREQ;
> > > > + prev->frequency = prev->driver_data;
> > >
> > > Okay you are using driver_data as a local variable to keep this value
> > > safe which you might have overwritten. Maybe use a simple variable
> > > prev_freq for this. It would be much more readable in that case and
> > > you wouldn't end up abusing the driver_data field.
> > >
>
> Please correct me, currently the driver_data is not used by cpufreq core and
> that was the reason to use it. In case you still think it is not a good way
> to handle it, I would try to handle it differently.
Yeah, the driver data will not be used by cpufreq core, but I think
the code would be far more readable if you use a local variable like
prev_freq instead.
> > > > + }
> > > > +
> > > > + break;
> > > > + }
> > > > + prev_cc = core_count;
> > > > + }
> > > > + c->table[i].frequency = CPUFREQ_TABLE_END;
> > >
> > > Wouldn't you end up writing on c->table[40].frequency here if there
> > > are 40 frequency value present ?
> >
> > Yeah, the loop condition needs to be fixed.
> >
>
> The table allocation is done for 'LUT_MAX_ENTRIES + 1'.
> Yes in case we have all [0-39] (i.e 40 entries) read from the hardware,
> would store the same and mark the 40th index as table end. Please correct if
> I missed something in your comment.
Ahh, you are right. I misread it.
--
viresh
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings
From: Viresh Kumar @ 2018-05-23 5:48 UTC (permalink / raw)
To: Rob Herring
Cc: Taniya Das, Rafael J. Wysocki, linux-kernel, linux-pm,
Stephen Boyd, Rajendra Nayak, Amit Nischal, devicetree, skannan,
amit.kucheria
In-Reply-To: <20180522193139.GA21623@rob-hp-laptop>
On 22-05-18, 14:31, Rob Herring wrote:
> On Sat, May 19, 2018 at 11:04:50PM +0530, Taniya Das wrote:
> > + freq-domain-0 {
> > + compatible = "cpufreq";
> > + reg = <0x17d43920 0x4>,
> > + <0x17d43110 0x500>,
> > + <0x17d41000 0x4>;
> > + reg-names = "perf_base", "lut_base", "en_base";
> > + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3>;
I was thinking, can't we add platform specific properties in the CPU
nodes ? If yes, then we can point the phandle of fw node from the CPUs
and this awkward list can go away.
--
viresh
^ permalink raw reply
* Re: [PATCH] cpufreq: Add Kryo CPU scaling driver
From: Viresh Kumar @ 2018-05-23 5:44 UTC (permalink / raw)
To: Sudeep Holla
Cc: Ilia Lin, linux-clk, devicetree, linux-kernel, linux-pm,
linux-arm-msm, linux-soc, linux-arm-kernel
In-Reply-To: <20180522130704.GA31065@e107155-lin>
On 22-05-18, 14:07, Sudeep Holla wrote:
> On Tue, May 22, 2018 at 02:29:45PM +0300, Ilia Lin wrote:
> > In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
> > the CPU frequency subset and voltage value of each OPP varies
> > based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables
> > defines the voltage and frequency value based on the msm-id in SMEM
> > and speedbin blown in the efuse combination.
> > The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
> > to provide the OPP framework with required information.
> > This is used to determine the voltage and frequency value for each OPP of
> > operating-points-v2 table when it is parsed by the OPP framework.
> >
> > Signed-off-by: Ilia Lin <ilialin@codeaurora.org>
> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> [...]
>
> > +
> > + switch (msm8996_version) {
> > + case MSM8996_V3:
> > + versions = 1 << (unsigned int)(*speedbin);
> > + break;
> > + case MSM8996_SG:
> > + versions = 1 << ((unsigned int)(*speedbin) + 4);
> > + break;
> > + default:
> > + BUG();
> > + break;
> > + }
> > +
> > + for_each_possible_cpu(cpu) {
> > + cpu_dev = get_cpu_device(cpu);
> > + if (NULL == cpu_dev) {
> > + ret = -ENODEV;
> > + goto free_opp;
> > + }
> > +
> > + opp_tables[cpu] = dev_pm_opp_set_supported_hw(cpu_dev,
> > + &versions, 1);
>
> Will be not get NULL for all CPUs except 0 ?
With my patches, we will get the OPP table again with refcount
incremented. And on failures, we need to call put-supported-hw helper
only for the CPUs for which it passed previously.
--
viresh
^ 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