* [PATCH v2 2/2] thermal/drivers/cpu_cooling: Unregister with the policy
From: Daniel Lezcano @ 2019-06-24 13:17 UTC (permalink / raw)
To: viresh.kumar
Cc: rjw, edubezval, linux-kernel, Sudeep Holla, Amit Daniel Kachhap,
Javi Merino, Zhang Rui, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team, Keerthy,
open list:CPU FREQUENCY DRIVERS - ARM BIG LITTLE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:TI BANDGAP AND THERMAL DRIVER
In-Reply-To: <20190624131715.1857-1-daniel.lezcano@linaro.org>
Currently the function cpufreq_cooling_register() returns a cooling
device pointer which is used back as a pointer to call the function
cpufreq_cooling_unregister(). Even if it is correct, it would make
sense to not leak the structure inside a cpufreq driver and keep the
code thermal code self-encapsulate. Moreover, that forces to add an
extra variable in each driver using this function.
Instead of passing the cooling device to unregister, pass the policy.
Because the cpufreq_cooling_unregister() function uses the policy to
unregister itself. The only purpose of the cooling device pointer is
to unregister the cpu cooling device.
As there is no more need of this pointer, remove it.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/arm_big_little.c | 9 +++------
drivers/cpufreq/cpufreq.c | 10 ++++------
drivers/thermal/cpu_cooling.c | 18 ++++++++++--------
drivers/thermal/imx_thermal.c | 12 ++++++------
.../thermal/ti-soc-thermal/ti-thermal-common.c | 10 +++++-----
include/linux/cpu_cooling.h | 6 +++---
include/linux/cpufreq.h | 3 ---
7 files changed, 31 insertions(+), 37 deletions(-)
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index 7fe52fcddcf1..718c63231e66 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -56,7 +56,6 @@ static bool bL_switching_enabled;
#define ACTUAL_FREQ(cluster, freq) ((cluster == A7_CLUSTER) ? freq << 1 : freq)
#define VIRT_FREQ(cluster, freq) ((cluster == A7_CLUSTER) ? freq >> 1 : freq)
-static struct thermal_cooling_device *cdev[MAX_CLUSTERS];
static const struct cpufreq_arm_bL_ops *arm_bL_ops;
static struct clk *clk[MAX_CLUSTERS];
static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS + 1];
@@ -501,10 +500,8 @@ static int bL_cpufreq_exit(struct cpufreq_policy *policy)
struct device *cpu_dev;
int cur_cluster = cpu_to_cluster(policy->cpu);
- if (cur_cluster < MAX_CLUSTERS) {
- cpufreq_cooling_unregister(cdev[cur_cluster]);
- cdev[cur_cluster] = NULL;
- }
+ if (cur_cluster < MAX_CLUSTERS)
+ cpufreq_cooling_unregister(policy);
cpu_dev = get_cpu_device(policy->cpu);
if (!cpu_dev) {
@@ -527,7 +524,7 @@ static void bL_cpufreq_ready(struct cpufreq_policy *policy)
if (cur_cluster >= MAX_CLUSTERS)
return;
- cdev[cur_cluster] = of_cpufreq_cooling_register(policy);
+ of_cpufreq_cooling_register(policy);
}
static struct cpufreq_driver bL_cpufreq_driver = {
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index aee024e42618..f07454249fbc 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1379,8 +1379,8 @@ static int cpufreq_online(unsigned int cpu)
cpufreq_driver->ready(policy);
if (cpufreq_thermal_control_enabled(cpufreq_driver))
- policy->cdev = of_cpufreq_cooling_register(policy);
-
+ of_cpufreq_cooling_register(policy);
+
pr_debug("initialization complete\n");
return 0;
@@ -1468,10 +1468,8 @@ static int cpufreq_offline(unsigned int cpu)
goto unlock;
}
- if (cpufreq_thermal_control_enabled(cpufreq_driver)) {
- cpufreq_cooling_unregister(policy->cdev);
- policy->cdev = NULL;
- }
+ if (cpufreq_thermal_control_enabled(cpufreq_driver))
+ cpufreq_cooling_unregister(policy);
if (cpufreq_driver->stop_cpu)
cpufreq_driver->stop_cpu(policy);
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 83486775e593..007c7c6bf845 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -78,6 +78,7 @@ struct cpufreq_cooling_device {
struct cpufreq_policy *policy;
struct list_head node;
struct time_in_idle *idle_time;
+ struct thermal_cooling_device *cdev;
};
static DEFINE_IDA(cpufreq_ida);
@@ -606,6 +607,7 @@ __cpufreq_cooling_register(struct device_node *np,
goto remove_ida;
cpufreq_cdev->clipped_freq = get_state_freq(cpufreq_cdev, 0);
+ cpufreq_cdev->cdev = cdev;
mutex_lock(&cooling_list_lock);
/* Register the notifier for first cpufreq cooling device */
@@ -699,18 +701,18 @@ EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register);
*
* This interface function unregisters the "thermal-cpufreq-%x" cooling device.
*/
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
{
struct cpufreq_cooling_device *cpufreq_cdev;
bool last;
- if (!cdev)
- return;
-
- cpufreq_cdev = cdev->devdata;
-
mutex_lock(&cooling_list_lock);
- list_del(&cpufreq_cdev->node);
+ list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
+ if (cpufreq_cdev->policy == policy) {
+ list_del(&cpufreq_cdev->node);
+ break;
+ }
+ }
/* Unregister the notifier for the last cpufreq cooling device */
last = list_empty(&cpufreq_cdev_list);
mutex_unlock(&cooling_list_lock);
@@ -719,7 +721,7 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
CPUFREQ_POLICY_NOTIFIER);
- thermal_cooling_device_unregister(cdev);
+ thermal_cooling_device_unregister(cpufreq_cdev->cdev);
ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
kfree(cpufreq_cdev->idle_time);
kfree(cpufreq_cdev);
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index bb6754a5342c..021c0948b740 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -203,7 +203,6 @@ static struct thermal_soc_data thermal_imx7d_data = {
struct imx_thermal_data {
struct cpufreq_policy *policy;
struct thermal_zone_device *tz;
- struct thermal_cooling_device *cdev;
enum thermal_device_mode mode;
struct regmap *tempmon;
u32 c1, c2; /* See formula in imx_init_calib() */
@@ -656,6 +655,7 @@ MODULE_DEVICE_TABLE(of, of_imx_thermal_match);
static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
{
struct device_node *np;
+ struct thermal_cooling_device *cdev;
int ret;
data->policy = cpufreq_cpu_get(0);
@@ -667,9 +667,9 @@ static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
np = of_get_cpu_node(data->policy->cpu, NULL);
if (!np || !of_find_property(np, "#cooling-cells", NULL)) {
- data->cdev = cpufreq_cooling_register(data->policy);
- if (IS_ERR(data->cdev)) {
- ret = PTR_ERR(data->cdev);
+ cdev = cpufreq_cooling_register(data->policy);
+ if (IS_ERR(cdev)) {
+ ret = PTR_ERR(cdev);
cpufreq_cpu_put(data->policy);
return ret;
}
@@ -680,7 +680,7 @@ static int imx_thermal_register_legacy_cooling(struct imx_thermal_data *data)
static void imx_thermal_unregister_legacy_cooling(struct imx_thermal_data *data)
{
- cpufreq_cooling_unregister(data->cdev);
+ cpufreq_cooling_unregister(data->policy);
cpufreq_cpu_put(data->policy);
}
@@ -872,7 +872,7 @@ static int imx_thermal_remove(struct platform_device *pdev)
clk_disable_unprepare(data->thermal_clk);
thermal_zone_device_unregister(data->tz);
- cpufreq_cooling_unregister(data->cdev);
+ cpufreq_cooling_unregister(data->policy);
cpufreq_cpu_put(data->policy);
return 0;
diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
index b4f981daeaf2..170b70b6ec61 100644
--- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
+++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
@@ -41,7 +41,6 @@ struct ti_thermal_data {
struct cpufreq_policy *policy;
struct thermal_zone_device *ti_thermal;
struct thermal_zone_device *pcb_tz;
- struct thermal_cooling_device *cool_dev;
struct ti_bandgap *bgp;
enum thermal_device_mode mode;
struct work_struct thermal_wq;
@@ -233,6 +232,7 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
{
struct ti_thermal_data *data;
struct device_node *np = bgp->dev->of_node;
+ struct thermal_cooling_device *cdev;
/*
* We are assuming here that if one deploys the zone
@@ -256,9 +256,9 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
}
/* Register cooling device */
- data->cool_dev = cpufreq_cooling_register(data->policy);
- if (IS_ERR(data->cool_dev)) {
- int ret = PTR_ERR(data->cool_dev);
+ cdev = cpufreq_cooling_register(data->policy);
+ if (IS_ERR(cdev)) {
+ int ret = PTR_ERR(cdev);
dev_err(bgp->dev, "Failed to register cpu cooling device %d\n",
ret);
cpufreq_cpu_put(data->policy);
@@ -277,7 +277,7 @@ int ti_thermal_unregister_cpu_cooling(struct ti_bandgap *bgp, int id)
data = ti_bandgap_get_sensor_data(bgp, id);
if (data) {
- cpufreq_cooling_unregister(data->cool_dev);
+ cpufreq_cooling_unregister(data->policy);
if (data->policy)
cpufreq_cpu_put(data->policy);
}
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index bae54bb7c048..89f469ee4be4 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -29,9 +29,9 @@ cpufreq_cooling_register(struct cpufreq_policy *policy);
/**
* cpufreq_cooling_unregister - function to remove cpufreq cooling device.
- * @cdev: thermal cooling device pointer.
+ * @policy: cpufreq policy
*/
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy);
#else /* !CONFIG_CPU_THERMAL */
static inline struct thermal_cooling_device *
@@ -41,7 +41,7 @@ cpufreq_cooling_register(struct cpufreq_policy *policy)
}
static inline
-void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
+void cpufreq_cooling_unregister(struct cpufreq_policy *policy)
{
return;
}
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 53ec82d75565..81ac78353f79 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -144,9 +144,6 @@ struct cpufreq_policy {
/* For cpufreq driver's internal use */
void *driver_data;
-
- /* Pointer to the cooling device if used for thermal mitigation */
- struct thermal_cooling_device *cdev;
};
struct cpufreq_freqs {
--
2.17.1
^ permalink raw reply related
* [PATCH v2 1/2] cpufreq: Move the IS_ENABLED(CPU_THERMAL) macro in a stub
From: Daniel Lezcano @ 2019-06-24 13:17 UTC (permalink / raw)
To: viresh.kumar
Cc: rjw, edubezval, linux-kernel,
open list:CPU FREQUENCY SCALING FRAMEWORK
The cpufreq_online and the cpufreq_offline [un]register the driver as
a cooling device. This is done if the driver is flagged as a cooling
device in addition with a IS_ENABLED macro to compile out the branching
code.
Group this test in a stub function added in the cpufreq header instead
of having the IS_ENABLED in the code path.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/cpufreq/cpufreq.c | 6 ++----
include/linux/cpufreq.h | 5 +++++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 85ff958e01f1..aee024e42618 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_thermal_control_enabled(cpufreq_driver))
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_thermal_control_enabled(cpufreq_driver)) {
cpufreq_cooling_unregister(policy->cdev);
policy->cdev = NULL;
}
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index d01a74fbc4db..53ec82d75565 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -409,6 +409,11 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
const char *cpufreq_get_current_driver(void);
void *cpufreq_get_driver_data(void);
+static inline int cpufreq_thermal_control_enabled(struct cpufreq_driver *drv)
+{
+ return IS_ENABLED(CPU_THERMAL) && (drv->flags & CPUFREQ_IS_COOLING_DEV);
+}
+
static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy,
unsigned int min, unsigned int max)
{
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2] PCI: PM: Skip devices in D0 for suspend-to-idle
From: Jon Hunter @ 2019-06-24 12:43 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PCI, Bjorn Helgaas
Cc: Linux PM, Linux ACPI, LKML, Mika Westerberg, Keith Busch,
Kai-Heng Feng, linux-tegra
In-Reply-To: <1668247.RaJIPSxJUN@kreacher>
Hi Rafael,
On 13/06/2019 22:59, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Commit d491f2b75237 ("PCI: PM: Avoid possible suspend-to-idle issue")
> attempted to avoid a problem with devices whose drivers want them to
> stay in D0 over suspend-to-idle and resume, but it did not go as far
> as it should with that.
>
> Namely, first of all, the power state of a PCI bridge with a
> downstream device in D0 must be D0 (based on the PCI PM spec r1.2,
> sec 6, table 6-1, if the bridge is not in D0, there can be no PCI
> transactions on its secondary bus), but that is not actively enforced
> during system-wide PM transitions, so use the skip_bus_pm flag
> introduced by commit d491f2b75237 for that.
>
> Second, the configuration of devices left in D0 (whatever the reason)
> during suspend-to-idle need not be changed and attempting to put them
> into D0 again by force is pointless, so explicitly avoid doing that.
>
> Fixes: d491f2b75237 ("PCI: PM: Avoid possible suspend-to-idle issue")
> Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
I have noticed a regression in both the mainline and -next branches on
one of our boards when testing suspend. The bisect is point to this
commit and reverting on top of mainline does fix the problem. So far I
have not looked at this in close detail but kernel log is showing ...
[ 52.775138] PM: suspend entry (deep)
[ 52.779040] Filesystems sync: 0.000 seconds
[ 52.783476] Freezing user space processes ... (elapsed 0.001 seconds) done.
[ 52.791891] OOM killer disabled.
[ 52.795174] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
[ 52.803752] printk: Suspending console(s) (use no_console_suspend to debug)
[ 52.823750] r8169 0000:01:00.0 eth0: Link is Down
[ 52.823908] pci_generic_config_write32: 22 callbacks suppressed
[ 52.823914] pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x9c may corrupt adjacent RW1C bits
[ 53.045383] Disabling non-boot CPUs ...
[ 53.048481] Entering suspend state LP1
[ 53.048508] Enabling non-boot CPUs ...
[ 53.049402] CPU1 is up
[ 53.050182] CPU2 is up
[ 53.051017] CPU3 is up
[ 53.051613] tegra-pcie 1003000.pcie: probing port 1, using 1 lanes
[ 53.069689] pcieport 0000:00:02.0: nv_msi_ht_cap_quirk didn't locate host bridge
[ 53.093751] r8169 0000:01:00.0: Refused to change power state, currently in D3
[ 53.156586] tegra-pcie 1003000.pcie: Slot present pin change, signature: 00000004
[ 53.156593] tegra-pcie 1003000.pcie: Response decoding error, signature: 10010045
[ 53.156596] tegra-pcie 1003000.pcie: FPCI address: fe10010044
[ 53.156711] tegra-pcie 1003000.pcie: Response decoding error, signature: 2000000c
[ 53.156714] tegra-pcie 1003000.pcie: FPCI address: 2000000c
[ 53.156719] tegra-pcie 1003000.pcie: Response decoding error, signature: 20000001
[ 53.156722] tegra-pcie 1003000.pcie: FPCI address: 20000000
[ 53.177372] pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x88 may corrupt adjacent RW1C bits
[ 53.177379] pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x90 may corrupt adjacent RW1C bits
[ 53.177384] pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x98 may corrupt adjacent RW1C bits
[ 53.177389] pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x9c may corrupt adjacent RW1C bits
[ 53.177394] pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0xa8 may corrupt adjacent RW1C bits
[ 53.177399] pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0xb0 may corrupt adjacent RW1C bits
[ 53.177451] pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x52 may corrupt adjacent RW1C bits
[ 53.177461] pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x52 may corrupt adjacent RW1C bits
[ 53.177470] pci_bus 0000:00: 2-byte config write to 0000:00:02.0 offset 0x5c may corrupt adjacent RW1C bits
[ 53.187746] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 53.188030] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.224105] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.224394] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.224679] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.224966] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.225247] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.225528] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.225813] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.226089] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.226372] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.226654] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.226934] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.227213] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.227495] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.227773] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.228050] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.228326] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.228609] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.228896] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 53.229181] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 54.064108] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 54.064429] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 54.064713] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 54.064996] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 54.065013] Generic Realtek PHY r8169-100:00: Master/Slave resolution failed, maybe conflicting manual settings?
[ 54.065016] ------------[ cut here ]------------
[ 54.065025] WARNING: CPU: 1 PID: 617 at /home/jonathanh/workdir/tegra/mlt-linux_torvalds/kernel/drivers/net/phy/phy.c:735 phy_error+0x1c/0x54
[ 54.065028] Modules linked in: ttm
[ 54.065039] CPU: 1 PID: 617 Comm: kworker/1:2 Not tainted 5.2.0-rc6 #1
[ 54.065041] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[ 54.065049] Workqueue: events_power_efficient phy_state_machine
[ 54.065068] [<c0112244>] (unwind_backtrace) from [<c010cad8>] (show_stack+0x10/0x14)
[ 54.065075] [<c010cad8>] (show_stack) from [<c0a60624>] (dump_stack+0xb4/0xc8)
[ 54.065082] [<c0a60624>] (dump_stack) from [<c0123cbc>] (__warn+0xe0/0xf8)
[ 54.065090] [<c0123cbc>] (__warn) from [<c0123dec>] (warn_slowpath_null+0x40/0x48)
[ 54.065095] [<c0123dec>] (warn_slowpath_null) from [<c06173f8>] (phy_error+0x1c/0x54)
[ 54.065101] [<c06173f8>] (phy_error) from [<c06184ec>] (phy_state_machine+0x64/0x1c0)
[ 54.065112] [<c06184ec>] (phy_state_machine) from [<c013e744>] (process_one_work+0x204/0x578)
[ 54.065119] [<c013e744>] (process_one_work) from [<c013f444>] (worker_thread+0x44/0x584)
[ 54.065123] [<c013f444>] (worker_thread) from [<c01445d4>] (kthread+0x148/0x150)
[ 54.065128] [<c01445d4>] (kthread) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
[ 54.065131] Exception stack(0xe91b3fb0 to 0xe91b3ff8)
[ 54.065134] 3fa0: 00000000 00000000 00000000 00000000
[ 54.065138] 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 54.065141] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 54.065145] ---[ end trace f59188238fc6fed4 ]---
[ 54.065168] r8169 0000:01:00.0 eth0: Link is Down
[ 54.075411] r8169 0000:01:00.0 eth0: rtl_chipcmd_cond == 1 (loop: 100, delay: 100).
[ 54.085652] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.095833] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.106017] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.116214] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.126419] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.136613] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.146829] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.157030] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.167223] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.177433] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.187635] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.197840] r8169 0000:01:00.0 eth0: rtl_eriar_cond == 1 (loop: 100, delay: 100).
[ 54.199008] r8169 0000:01:00.0 eth0: rtl_ephyar_cond == 1 (loop: 100, delay: 10).
[ 54.200173] r8169 0000:01:00.0 eth0: rtl_ephyar_cond == 1 (loop: 100, delay: 10).
[ 54.201336] r8169 0000:01:00.0 eth0: rtl_ephyar_cond == 1 (loop: 100, delay: 10).
[ 54.202502] r8169 0000:01:00.0 eth0: rtl_ephyar_cond == 1 (loop: 100, delay: 10).
[ 54.203018] r8169 0000:01:00.0 eth0: rtl_ocp_gphy_cond == 1 (loop: 10, delay: 25).
[ 54.546178] ata1: SATA link down (SStatus 0 SControl 300)
[ 56.810813] OOM killer enabled.
[ 56.813953] Restarting tasks ... done.
[ 56.819622] PM: suspend exit
[ 72.504381] nfs: server 192.168.99.1 not responding, still trying
[ 78.104369] nfs: server 192.168.99.1 not responding, still trying
Let me know if you have any thoughts.
Cheers
Jon
--
nvpublic
^ permalink raw reply
* Re: Alternatives to /sys/kernel/debug/wakeup_sources
From: Joel Fernandes @ 2019-06-24 12:27 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Tri Vo, Rafael J. Wysocki, Rafael J. Wysocki, Sandeep Patil,
Viresh Kumar, Hridya Valsaraju, Linux PM, Cc: Android Kernel,
LKML
In-Reply-To: <20190624073659.GA13957@kroah.com>
On Mon, Jun 24, 2019 at 3:37 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sun, Jun 23, 2019 at 06:48:43PM -0700, Tri Vo wrote:
> > On Wed, Jun 19, 2019 at 1:35 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > On Wed, Jun 19, 2019 at 1:52 AM Joel Fernandes <joelaf@google.com> wrote:
> > > >
> > > > On Tue, Jun 18, 2019 at 7:15 PM Tri Vo <trong@android.com> wrote:
> > > > [snip]
> > > > > > > > >
> > > > > > > > > Android userspace reading wakeup_sources is not ideal because:
> > > > > > > > > - Debugfs API is not stable, i.e. Android tools built on top of it are
> > > > > > > > > not guaranteed to be backward/forward compatible.
> > > > > > > > > - This file requires debugfs to be mounted, which itself is
> > > > > > > > > undesirable for security reasons.
> > > > > > > > >
> > > > > > > > > To address these problems, we want to contribute a way to expose these
> > > > > > > > > statistics that doesn't depend on debugfs.
> > > > > > > > >
> > > > > > > > > Some initial thoughts/questions: Should we expose the stats in sysfs?
> > > > > > > > > Or maybe implement eBPF-based solution? What do you think?
> > > > > > >
> > > > > > > We are going through Android's out-of-tree kernel dependencies along with
> > > > > > > userspace APIs that are not necessarily considered "stable and forever
> > > > > > > supported" upstream. The debugfs dependencies showed up on our radar as a
> > > > > > > result and so we are wondering if we should worry about changes in debugfs
> > > > > > > interface and hence the question(s) below.
> > > > > > >
> > > > > > > So, can we rely on /d/wakeup_sources to be considered a userspace API and
> > > > > > > hence maintained stable as we do for other /proc and /sys entries?
> > > > > > >
> > > > > > > If yes, then we will go ahead and add tests for this in LTP or
> > > > > > > somewhere else suitable.
> > > > > >
> > > > > > No, debugfs is not ABI.
> > > > > >
> > > > > > > If no, then we would love to hear suggestions for any changes that need to be
> > > > > > > made or we simply just move the debugfs entry into somewhere like
> > > > > > > /sys/power/ ?
> > > > > >
> > > > > > No, moving that entire file from debugfs into sysfs is not an option either.
> > > > > >
> > > > > > The statistics for the wakeup sources associated with devices are already there
> > > > > > under /sys/devices/.../power/ , but I guess you want all wakeup sources?
> > > > > >
> > > > > > That would require adding a kobject to struct wakeup_source and exposing
> > > > > > all of the statistics as separate attributes under it. In which case it would be
> > > > > > good to replace the existing wakeup statistics under /sys/devices/.../power/
> > > > > > with symbolic links to the attributes under the wakeup_source kobject.
> > > > >
> > > > > Thanks for your input, Rafael! Your suggestion makes sense. I'll work
> > > > > on a patch for this.
> > > >
> > > > Does that entail making each wake up source, a new sysfs node under a
> > > > particular device, and then adding stats under that new node?
> > >
> > > Not under a device, because there are wakeup source objects without
> > > associated devices.
> > >
> > > It is conceivable to have a "wakeup_sources" directory under
> > > /sys/power/ and sysfs nodes for all wakeup sources in there.
> > >
> > > Then, instead of exposing wakeup statistics directly under
> > > /sys/devices/.../power/, there can be symbolic links from there to the
> > > new wakeup source nodes under "wakeup_sources" (so as to avoid
> > > exposing the same data in two different places in sysfs, which may be
> > > confusing).
> >
> > This may be a dumb question. Is it appropriate to make symbolic links
> > in sysfs from one attribute to another attribute? For example,
> > /sys/devices/.../power/wakeup_count ->
> > /sys/power/wakeup_sources/.../wakeup_count.
>
> Why? would you want that?
This sounds like what Rafael suggested (quoted above), right?
^ permalink raw reply
* Re: [PATCH v4 01/11] thermal: sun8i: add thermal driver for h6
From: Ondřej Jirman @ 2019-06-24 12:07 UTC (permalink / raw)
To: Yangtao Li
Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland,
maxime.ripard, wens, davem, gregkh, mchehab+samsung,
linus.walleij, nicolas.ferre, paulmck, devicetree, linux-kernel,
linux-arm-kernel, linux-pm
In-Reply-To: <20190623164206.7467-2-tiny.windzz@gmail.com>
Hello Yangtao,
On Sun, Jun 23, 2019 at 12:41:56PM -0400, Yangtao Li wrote:
> This patch adds the support for allwinner thermal sensor, within
> allwinner SoC. It will register sensors for thermal framework
> and use device tree to bind cooling device.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
> MAINTAINERS | 7 +
> drivers/thermal/Kconfig | 14 ++
> drivers/thermal/Makefile | 1 +
> drivers/thermal/sun8i_thermal.c | 405 ++++++++++++++++++++++++++++++++
> 4 files changed, 427 insertions(+)
> create mode 100644 drivers/thermal/sun8i_thermal.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 36a84614d6c3..67e7fcfaded2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -674,6 +674,13 @@ L: linux-crypto@vger.kernel.org
> S: Maintained
> F: drivers/crypto/sunxi-ss/
>
> +ALLWINNER THERMAL DRIVER
> +M: Yangtao Li <tiny.windzz@gmail.com>
> +L: linux-pm@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml
> +F: drivers/thermal/sun8i_thermal.c
> +
> ALLWINNER VPU DRIVER
> M: Maxime Ripard <maxime.ripard@bootlin.com>
> M: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 9966364a6deb..f8b73b32b92d 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -262,6 +262,20 @@ config SPEAR_THERMAL
> Enable this to plug the SPEAr thermal sensor driver into the Linux
> thermal framework.
>
> +config SUN8I_THERMAL
> + tristate "Allwinner sun8i thermal driver"
> + depends on ARCH_SUNXI || COMPILE_TEST
> + depends on HAS_IOMEM
> + depends on NVMEM
> + depends on OF
> + depends on RESET_CONTROLLER
> + help
> + Support for the sun8i thermal sensor driver into the Linux thermal
> + framework.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called sun8i-thermal.
> +
> config ROCKCHIP_THERMAL
> tristate "Rockchip thermal driver"
> depends on ARCH_ROCKCHIP || COMPILE_TEST
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 74a37c7f847a..fa6f8b206281 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -31,6 +31,7 @@ thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
> obj-y += broadcom/
> obj-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o
> obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
> +obj-$(CONFIG_SUN8I_THERMAL) += sun8i_thermal.o
> obj-$(CONFIG_ROCKCHIP_THERMAL) += rockchip_thermal.o
> obj-$(CONFIG_RCAR_THERMAL) += rcar_thermal.o
> obj-$(CONFIG_RCAR_GEN3_THERMAL) += rcar_gen3_thermal.o
> diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
> new file mode 100644
> index 000000000000..d6918c62682b
> --- /dev/null
> +++ b/drivers/thermal/sun8i_thermal.c
> @@ -0,0 +1,405 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Thermal sensor driver for Allwinner SOC
> + * Copyright (C) 2019 Yangtao Li
> + *
> + * Based on the work of Icenowy Zheng <icenowy@aosc.io>
> + * Based on the work of Ondrej Jirman <megous@megous.com>
> + * Based on the work of Josef Gajdusek <atx@atx.name>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +#include <linux/thermal.h>
> +
> +#define MAX_SENSOR_NUM 4
> +
> +#define SUN50I_H6_SENSOR_NUM 2
> +#define SUN50I_H6_OFFSET -2794
> +#define SUN50I_H6_SCALE -67
> +
> +#define FT_TEMP_MASK GENMASK(11, 0)
> +#define TEMP_CALIB_MASK GENMASK(11, 0)
> +#define TEMP_TO_REG 672
> +#define CALIBRATE_DEFAULT 0x800
> +
> +#define SUN50I_THS_CTRL0 0x00
> +#define SUN50I_H6_THS_ENABLE 0x04
> +#define SUN50I_H6_THS_PC 0x08
> +#define SUN50I_H6_THS_DIC 0x10
> +#define SUN50I_H6_THS_DIS 0x20
> +#define SUN50I_H6_THS_MFC 0x30
> +#define SUN50I_H6_THS_TEMP_CALIB 0xa0
> +#define SUN50I_H6_THS_TEMP_DATA 0xc0
> +
> +#define SUN50I_THS_CTRL0_T_ACQ(x) ((GENMASK(15, 0) & (x)) << 16)
> +#define SUN50I_THS_FILTER_EN BIT(2)
> +#define SUN50I_THS_FILTER_TYPE(x) (GENMASK(1, 0) & (x))
> +#define SUN50I_H6_THS_PC_TEMP_PERIOD(x) ((GENMASK(19, 0) & (x)) << 12)
> +#define SUN50I_H6_THS_DATA_IRQ_STS(x) BIT(x)
> +
> +/* millidegree celsius */
> +#define SUN50I_H6_FT_DEVIATION 7000
> +
> +struct ths_device;
> +
> +struct tsensor {
> + struct ths_device *tmdev;
> + struct thermal_zone_device *tzd;
> + int id;
> +};
> +
> +struct ths_device {
> + struct device *dev;
> + struct regmap *regmap;
> + struct reset_control *reset;
> + struct clk *bus_clk;
> + struct tsensor sensor[MAX_SENSOR_NUM];
> +};
> +
> +/* Temp Unit: millidegree Celsius */
> +static int sun8i_ths_reg2temp(struct ths_device *tmdev,
> + int reg)
> +{
> + return (reg + SUN50I_H6_OFFSET) * SUN50I_H6_SCALE;
> +}
> +
> +static int sun8i_ths_get_temp(void *data, int *temp)
> +{
> + struct tsensor *s = data;
> + struct ths_device *tmdev = s->tmdev;
> + int val;
> +
> + regmap_read(tmdev->regmap, SUN50I_H6_THS_TEMP_DATA +
> + 0x4 * s->id, &val);
> +
> + /* ths have no data yet */
> + if (!val)
> + return -EBUSY;
You should return -EAGAIN here to avoid the warning (failed to read out thermal
zone (%d)) if you believe this error is temporary.
regards,
o.
> + *temp = sun8i_ths_reg2temp(tmdev, val);
> + /*
> + * XX - According to the original sdk, there are some platforms(rarely)
> + * that add a fixed offset value after calculating the temperature
> + * value. We can't simply put it on the formula for calculating the
> + * temperature above, because the formula for calculating the
> + * temperature above is also used when the sensor is calibrated. If
> + * do this, the correct calibration formula is hard to know.
> + */
> + *temp += SUN50I_H6_FT_DEVIATION;
> +
> + return 0;
> +}
> +
> +static const struct thermal_zone_of_device_ops ths_ops = {
> + .get_temp = sun8i_ths_get_temp,
> +};
> +
> +static const struct regmap_config config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .fast_io = true,
> +};
> +
> +static irqreturn_t sun50i_h6_irq_thread(int irq, void *data)
> +{
> + struct ths_device *tmdev = data;
> + int i, state;
> +
> + regmap_read(tmdev->regmap, SUN50I_H6_THS_DIS, &state);
> +
> + for (i = 0; i < SUN50I_H6_SENSOR_NUM; i++) {
> +
> + if (state & SUN50I_H6_THS_DATA_IRQ_STS(i)) {
> + /* clear data irq pending */
> + regmap_write(tmdev->regmap, SUN50I_H6_THS_DIS,
> + SUN50I_H6_THS_DATA_IRQ_STS(i));
> +
> + thermal_zone_device_update(tmdev->sensor[i].tzd,
> + THERMAL_EVENT_UNSPECIFIED);
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int sun50i_ths_calibrate(struct ths_device *tmdev)
> +{
> + struct nvmem_cell *calcell;
> + struct device *dev = tmdev->dev;
> + u16 *caldata;
> + size_t callen;
> + int ft_temp;
> + int i, ret = 0;
> +
> + calcell = devm_nvmem_cell_get(dev, "calib");
> + if (IS_ERR(calcell)) {
> + if (PTR_ERR(calcell) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + /*
> + * Even if the external calibration data stored in sid is
> + * not accessible, the THS hardware can still work, although
> + * the data won't be so accurate.
> + *
> + * The default value of calibration register is 0x800 for
> + * every sensor, and the calibration value is usually 0x7xx
> + * or 0x8xx, so they won't be away from the default value
> + * for a lot.
> + *
> + * So here we do not return error if the calibartion data is
> + * not available, except the probe needs deferring.
> + */
> + goto out;
> + }
> +
> + caldata = nvmem_cell_read(calcell, &callen);
> + if (IS_ERR(caldata)) {
> + ret = PTR_ERR(caldata);
> + goto out;
> + }
> +
> + if (!caldata[0] || callen < 2 + 2 * SUN50I_H6_SENSOR_NUM) {
> + ret = -EINVAL;
> + goto out_free;
> + }
> +
> + /*
> + * efuse layout:
> + *
> + * 0 11 16 32
> + * +-------+-------+-------+
> + * |temp| |sensor0|sensor1|
> + * +-------+-------+-------+
> + *
> + * The calibration data on the H6 is the ambient temperature and
> + * sensor values that are filled during the factory test stage.
> + *
> + * The unit of stored FT temperature is 0.1 degreee celusis.
> + * Through the stored ambient temperature and the data read
> + * by the sensor, after a certain calculation, the calibration
> + * value to be compensated can be obtained.
> + */
> + ft_temp = caldata[0] & FT_TEMP_MASK;
> +
> + for (i = 0; i < SUN50I_H6_SENSOR_NUM; i++) {
> + int reg = (int)caldata[i + 1];
> + int sensor_temp = sun8i_ths_reg2temp(tmdev, reg);
> + int delta, cdata, calib_offest;
> +
> + /*
> + * To calculate the calibration value:
> + *
> + * X(in Celsius) = Ts - ft_temp
> + * delta = X * 10000 / TEMP_TO_REG
> + * cdata = CALIBRATE_DEFAULT - delta
> + *
> + * cdata: calibration value
> + */
> + delta = (sensor_temp - ft_temp * 100) * 10 / TEMP_TO_REG;
> + cdata = CALIBRATE_DEFAULT - delta;
> + if (cdata & ~TEMP_CALIB_MASK) {
> + /*
> + * Calibration value more than 12-bit, but calibration
> + * register is 12-bit. In this case, ths hardware can
> + * still work without calibration, although the data
> + * won't be so accurate.
> + */
> + dev_warn(dev, "sensor%d is not calibrated.\n", i);
> +
> + continue;
> + }
> +
> + calib_offest = SUN50I_H6_THS_TEMP_CALIB + (i / 2) * 0x4;
> +
> + if (i % 2) {
> + int val;
> +
> + regmap_read(tmdev->regmap, calib_offest, &val);
> + val = (val & TEMP_CALIB_MASK) | (cdata << 16);
> + regmap_write(tmdev->regmap, calib_offest, val);
> + } else {
> + regmap_write(tmdev->regmap, calib_offest, cdata);
> + }
> + }
> +
> +out_free:
> + kfree(caldata);
> +out:
> + return ret;
> +}
> +
> +static int sun8i_ths_resource_init(struct ths_device *tmdev)
> +{
> + struct device *dev = tmdev->dev;
> + struct platform_device *pdev = to_platform_device(dev);
> + struct resource *mem;
> + void __iomem *base;
> + int ret;
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(dev, mem);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + tmdev->regmap = devm_regmap_init_mmio(dev, base, &config);
> + if (IS_ERR(tmdev->regmap))
> + return PTR_ERR(tmdev->regmap);
> +
> + tmdev->reset = devm_reset_control_get(dev, 0);
> + if (IS_ERR(tmdev->reset))
> + return PTR_ERR(tmdev->reset);
> +
> + tmdev->bus_clk = devm_clk_get(&pdev->dev, "bus");
> + if (IS_ERR(tmdev->bus_clk))
> + return PTR_ERR(tmdev->bus_clk);
> +
> + ret = reset_control_deassert(tmdev->reset);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(tmdev->bus_clk);
> + if (ret)
> + goto assert_reset;
> +
> + ret = sun50i_ths_calibrate(tmdev);
> + if (ret)
> + goto bus_disable;
> +
> + return 0;
> +
> +bus_disable:
> + clk_disable_unprepare(tmdev->bus_clk);
> +assert_reset:
> + reset_control_assert(tmdev->reset);
> +
> + return ret;
> +}
> +
> +static int sun50i_thermal_init(struct ths_device *tmdev)
> +{
> + int val;
> +
> + /*
> + * clkin = 24MHz
> + * T acquire = clkin / (x + 1)
> + * = 20us
> + */
> + regmap_write(tmdev->regmap, SUN50I_THS_CTRL0,
> + SUN50I_THS_CTRL0_T_ACQ(479));
> + /* average over 4 samples */
> + regmap_write(tmdev->regmap, SUN50I_H6_THS_MFC,
> + SUN50I_THS_FILTER_EN |
> + SUN50I_THS_FILTER_TYPE(1));
> + /* period = (x + 1) * 4096 / clkin; ~10ms */
> + regmap_write(tmdev->regmap, SUN50I_H6_THS_PC,
> + SUN50I_H6_THS_PC_TEMP_PERIOD(58));
> + /* enable sensor */
> + val = GENMASK(SUN50I_H6_SENSOR_NUM - 1, 0);
> + regmap_write(tmdev->regmap, SUN50I_H6_THS_ENABLE, val);
> + /* thermal data interrupt enable */
> + val = GENMASK(SUN50I_H6_SENSOR_NUM - 1, 0);
> + regmap_write(tmdev->regmap, SUN50I_H6_THS_DIC, val);
> +
> + return 0;
> +}
> +
> +static int sun8i_ths_register(struct ths_device *tmdev)
> +{
> + struct thermal_zone_device *tzd;
> + int i;
> +
> + for (i = 0; i < SUN50I_H6_SENSOR_NUM; i++) {
> + tmdev->sensor[i].tmdev = tmdev;
> + tmdev->sensor[i].id = i;
> + tmdev->sensor[i].tzd =
> + devm_thermal_zone_of_sensor_register(tmdev->dev,
> + i,
> + &tmdev->sensor[i],
> + &ths_ops);
> + if (IS_ERR(tmdev->sensor[i].tzd))
> + return PTR_ERR(tzd);
> + }
> +
> + return 0;
> +}
> +
> +static int sun8i_ths_probe(struct platform_device *pdev)
> +{
> + struct ths_device *tmdev;
> + struct device *dev = &pdev->dev;
> + int ret, irq;
> +
> + tmdev = devm_kzalloc(dev, sizeof(*tmdev), GFP_KERNEL);
> + if (!tmdev)
> + return -ENOMEM;
> +
> + tmdev->dev = dev;
> + platform_set_drvdata(pdev, tmdev);
> +
> + ret = sun8i_ths_resource_init(tmdev);
> + if (ret)
> + return ret;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + ret = sun50i_thermal_init(tmdev);
> + if (ret)
> + return ret;
> +
> + ret = sun8i_ths_register(tmdev);
> + if (ret)
> + return ret;
> +
> + /*
> + * Avoid entering the interrupt handler, the thermal device is not
> + * registered yet, we deffer the registration of the interrupt to
> + * the end.
> + */
> + ret = devm_request_threaded_irq(dev, irq, NULL,
> + sun50i_h6_irq_thread,
> + IRQF_ONESHOT, "ths", tmdev);
> + if (ret)
> + return ret;
> +
> + return ret;
> +}
> +
> +static int sun8i_ths_remove(struct platform_device *pdev)
> +{
> + struct ths_device *tmdev = platform_get_drvdata(pdev);
> +
> + clk_disable_unprepare(tmdev->bus_clk);
> + reset_control_assert(tmdev->reset);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id of_ths_match[] = {
> + { .compatible = "allwinner,sun50i-h6-ths"},
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, of_ths_match);
> +
> +static struct platform_driver ths_driver = {
> + .probe = sun8i_ths_probe,
> + .remove = sun8i_ths_remove,
> + .driver = {
> + .name = "sun8i-thermal",
> + .of_match_table = of_ths_match,
> + },
> +};
> +module_platform_driver(ths_driver);
> +
> +MODULE_DESCRIPTION("Thermal sensor driver for Allwinner SOC");
> +MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 11/11] thermal: sun8i: add thermal driver for h3
From: Ondřej Jirman @ 2019-06-24 12:05 UTC (permalink / raw)
To: Yangtao Li
Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland,
maxime.ripard, wens, davem, gregkh, mchehab+samsung,
linus.walleij, nicolas.ferre, paulmck, devicetree, linux-kernel,
linux-arm-kernel, linux-pm
In-Reply-To: <20190623164206.7467-12-tiny.windzz@gmail.com>
Hello Yangtao,
On Sun, Jun 23, 2019 at 12:42:06PM -0400, Yangtao Li wrote:
> This patch adds the support for allwinner h3 thermal sensor.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
> drivers/thermal/sun8i_thermal.c | 72 +++++++++++++++++++++++++++++++++
> 1 file changed, 72 insertions(+)
>
> diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
> index 260b24340f5b..c8ee291f3b17 100644
> --- a/drivers/thermal/sun8i_thermal.c
> +++ b/drivers/thermal/sun8i_thermal.c
> @@ -27,6 +27,14 @@
> #define TEMP_TO_REG 672
> #define CALIBRATE_DEFAULT 0x800
>
> +#define SUN8I_THS_CTRL0 0x00
> +#define SUN8I_THS_CTRL2 0x40
> +#define SUN8I_THS_IC 0x44
> +#define SUN8I_THS_IS 0x48
> +#define SUN8I_THS_MFC 0x70
> +#define SUN8I_THS_TEMP_CALIB 0x74
> +#define SUN8I_THS_TEMP_DATA 0x80
> +
> #define SUN50I_THS_CTRL0 0x00
> #define SUN50I_H6_THS_ENABLE 0x04
> #define SUN50I_H6_THS_PC 0x08
> @@ -36,6 +44,9 @@
> #define SUN50I_H6_THS_TEMP_CALIB 0xa0
> #define SUN50I_H6_THS_TEMP_DATA 0xc0
>
> +#define SUN8I_THS_CTRL0_T_ACQ0(x) (GENMASK(15, 0) & (x))
> +#define SUN8I_THS_CTRL2_T_ACQ1(x) ((GENMASK(15, 0) & (x)) << 16)
> +
> #define SUN50I_THS_CTRL0_T_ACQ(x) ((GENMASK(15, 0) & (x)) << 16)
> #define SUN50I_THS_FILTER_EN BIT(2)
> #define SUN50I_THS_FILTER_TYPE(x) (GENMASK(1, 0) & (x))
> @@ -121,6 +132,21 @@ static const struct regmap_config config = {
> .fast_io = true,
> };
>
> +static int sun8i_h3_irq_ack(struct ths_device *tmdev)
> +{
> + int state, ret = 0;
> +
> + regmap_read(tmdev->regmap, SUN8I_THS_IS, &state);
> +
> + if (state & BIT(8)) {
> + regmap_write(tmdev->regmap, SUN8I_THS_IS,
> + BIT(8));
> + ret |= BIT(1);
> + }
> +
> + return ret;
> +}
> +
> static int sun50i_h6_irq_ack(struct ths_device *tmdev)
> {
> int i, state, ret = 0;
> @@ -154,6 +180,14 @@ static irqreturn_t sun8i_irq_thread(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static int sun8i_h3_ths_calibrate(struct ths_device *tmdev,
> + u16 *caldata, int callen)
> +{
> + regmap_write(tmdev->regmap, SUN8I_THS_TEMP_CALIB, *caldata);
You're missing a sanity check for callen here.
regards,
o.
> + return 0;
> +}
> +
> static int sun50i_h6_ths_calibrate(struct ths_device *tmdev,
> u16 *caldata, int callen)
> {
> @@ -325,6 +359,32 @@ static int sun8i_ths_resource_init(struct ths_device *tmdev)
> return ret;
> }
>
> +static int sun8i_h3_thermal_init(struct ths_device *tmdev)
> +{
> + /* average over 4 samples */
> + regmap_write(tmdev->regmap, SUN8I_THS_MFC,
> + SUN50I_THS_FILTER_EN |
> + SUN50I_THS_FILTER_TYPE(1));
> + /*
> + * period = (x + 1) * 4096 / clkin; ~10ms
> + * enable data interrupt
> + */
> + regmap_write(tmdev->regmap, SUN8I_THS_IC,
> + SUN50I_H6_THS_PC_TEMP_PERIOD(58) | BIT(8));
> + /*
> + * clkin = 24MHz
> + * T acquire = clkin / (x + 1)
> + * = 20us
> + * enable sensor
> + */
> + regmap_write(tmdev->regmap, SUN8I_THS_CTRL0,
> + SUN8I_THS_CTRL0_T_ACQ0(479));
> + regmap_write(tmdev->regmap, SUN8I_THS_CTRL2,
> + SUN8I_THS_CTRL2_T_ACQ1(479) | BIT(0));
> +
> + return 0;
> +}
> +
> static int sun50i_thermal_init(struct ths_device *tmdev)
> {
> int val;
> @@ -431,6 +491,17 @@ static int sun8i_ths_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct ths_thermal_chip sun8i_h3_ths = {
> + .sensor_num = 1,
> + .offset = -1794,
> + .scale = -121,
> + .has_ahb_clk = true,
> + .temp_data_base = SUN8I_THS_TEMP_DATA,
> + .calibrate = sun8i_h3_ths_calibrate,
> + .init = sun8i_h3_thermal_init,
> + .irq_ack = sun8i_h3_irq_ack,
> +};
> +
> static const struct ths_thermal_chip sun50i_h6_ths = {
> .sensor_num = 2,
> .offset = -2794,
> @@ -443,6 +514,7 @@ static const struct ths_thermal_chip sun50i_h6_ths = {
> };
>
> static const struct of_device_id of_ths_match[] = {
> + { .compatible = "allwinner,sun8i-h3-ths", .data = &sun8i_h3_ths },
> { .compatible = "allwinner,sun50i-h6-ths", .data = &sun50i_h6_ths },
> { /* sentinel */ },
> };
> --
> 2.17.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 13/16] PM / devfreq: tegra: Support Tegra30
From: Dmitry Osipenko @ 2019-06-24 11:24 UTC (permalink / raw)
To: myungjoo.ham
Cc: Chanwoo Choi, linux-pm@vger.kernel.org,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
jonathanh@nvidia.com, thierry.reding@gmail.com
In-Reply-To: <20190624111134epcms1p361aed3c72edd6eebc95408331c8d9739@epcms1p3>
24.06.2019 14:11, MyungJoo Ham пишет:
>>
>> --------- Original Message ---------
>> Sender : Dmitry Osipenko <digetx@gmail.com>
>>
>> 24.06.2019 10:34, MyungJoo Ham пишет:
>>>>
>>>> A question:
>>>>
>>>> Does this driver support Tegra20 as well?
>>>> I'm asking this because ARCH_TEGRA includes ARCH_TEGRA_2x_SOC
>>>> according to /drivers/soc/tegra/Kconfig.
>>>>
>>>
>>> For this matter, how about updating your 13/16 patch as follows?
>>>
> []
>>
>> Good call! I'll update this patch following yours suggestion, thanks.
>
> Or, you may approve the modified commits here:
> https://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git/log/?h=for-next
Looks almost good to me!
I just recalled that there is also a 64bit variant of Tegra124, the Tegra132. Hence
the Tegra30+ Kconfig entry should look like this (it's also worthy to break the lines
for readability):
diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index ccb1a68c4b51..bd2efbc27725 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -94,7 +94,10 @@ config ARM_EXYNOS_BUS_DEVFREQ
config ARM_TEGRA_DEVFREQ
tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
- depends on ARCH_TEGRA || COMPILE_TEST
+ depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
+ ARCH_TEGRA_132_SOC || ARCH_TEGRA_124_SOC || \
+ ARCH_TEGRA_210_SOC || \
+ COMPILE_TEST
select PM_OPP
help
This adds the DEVFREQ driver for the Tegra family of SoCs.
Could you please adjust the patches like I'm suggesting? I'll approve yours change
then and won't re-spin the first batch of the patches.
^ permalink raw reply related
* RE: Re: [PATCH v4 13/16] PM / devfreq: tegra: Support Tegra30
From: MyungJoo Ham @ 2019-06-24 11:11 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Chanwoo Choi, linux-pm@vger.kernel.org,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
jonathanh@nvidia.com, thierry.reding@gmail.com
In-Reply-To: <37db00bc-3a22-d1c2-7bdc-e27af42cd5c7@gmail.com>
>
>--------- Original Message ---------
>Sender : Dmitry Osipenko <digetx@gmail.com>
>
>24.06.2019 10:34, MyungJoo Ham пишет:
>>>
>>> A question:
>>>
>>> Does this driver support Tegra20 as well?
>>> I'm asking this because ARCH_TEGRA includes ARCH_TEGRA_2x_SOC
>>> according to /drivers/soc/tegra/Kconfig.
>>>
>>
>> For this matter, how about updating your 13/16 patch as follows?
>>
[]
>
>Good call! I'll update this patch following yours suggestion, thanks.
Or, you may approve the modified commits here:
https://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git/log/?h=for-next
Cheers,
MyungJoo
^ permalink raw reply
* Re: [PATCH v4 16/16] PM / devfreq: Introduce driver for NVIDIA Tegra20
From: Dmitry Osipenko @ 2019-06-24 10:48 UTC (permalink / raw)
To: myungjoo.ham, thierry.reding@gmail.com, jonathanh@nvidia.com,
Chanwoo Choi, linux-pm@vger.kernel.org,
linux-tegra@vger.kernel.org
In-Reply-To: <20190624072431epcms1p3bdfd41545e7daecb1f6472c1e6f9dcfc@epcms1p3>
24.06.2019 10:24, MyungJoo Ham пишет:
>> Add devfreq driver for NVIDIA Tegra20 SoC's. The driver periodically
>> reads out Memory Controller counters and adjusts memory frequency based
>> on the memory clients activity.
>>
>> Reviewed-by: Chanwoo Choi
>> Signed-off-by: Dmitry Osipenko
>
> Could you please send a separate commit for MAINTAINERS?
>
> I can add Ack to it, but I don't feel comfortable with sending
> a pull-request with MAINTAINERS entry updates.
Okay, I'll factor out the MAINTAINERS entry update into a separate patch.
I'll re-spin this series, appending the followup patches [1], and will
also squash the Tegra20 / Tegra30 MAINTAINERS update into a single patch.
[1] https://lore.kernel.org/lkml/20190623214658.11680-1-digetx@gmail.com/T/
^ permalink raw reply
* Re: [PATCH v4 13/16] PM / devfreq: tegra: Support Tegra30
From: Dmitry Osipenko @ 2019-06-24 10:42 UTC (permalink / raw)
To: myungjoo.ham
Cc: Chanwoo Choi, linux-pm@vger.kernel.org,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
jonathanh@nvidia.com, thierry.reding@gmail.com
In-Reply-To: <20190624073414epcms1p87b6dc13758b6bd401d275cfba583314a@epcms1p8>
24.06.2019 10:34, MyungJoo Ham пишет:
>>
>> A question:
>>
>> Does this driver support Tegra20 as well?
>> I'm asking this because ARCH_TEGRA includes ARCH_TEGRA_2x_SOC
>> according to /drivers/soc/tegra/Kconfig.
>>
>
> For this matter, how about updating your 13/16 patch as follows?
>
> ---
> drivers/devfreq/Kconfig | 4 ++--
> drivers/devfreq/tegra-devfreq.c | 1 +
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 7dd46d44579d..78c4b436aad8 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -93,8 +93,8 @@ config ARM_EXYNOS_BUS_DEVFREQ
> This does not yet operate with optimal voltages.
>
> config ARM_TEGRA_DEVFREQ
> - tristate "Tegra DEVFREQ Driver"
> - depends on ARCH_TEGRA_124_SOC
> + tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
> + depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || ARCH_TEGRA_124_SOC || ARCH_TEGRA_210_SOC
> select PM_OPP
> help
> This adds the DEVFREQ driver for the Tegra family of SoCs.
> diff --git a/drivers/devfreq/tegra-devfreq.c b/drivers/devfreq/tegra-devfreq.c
> index 5cddf2199c4e..a6ba75f4106d 100644
> --- a/drivers/devfreq/tegra-devfreq.c
> +++ b/drivers/devfreq/tegra-devfreq.c
> @@ -726,6 +726,7 @@ static int tegra_devfreq_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id tegra_devfreq_of_match[] = {
> + { .compatible = "nvidia,tegra30-actmon" },
> { .compatible = "nvidia,tegra124-actmon" },
> { },
> };
>
Good call! I'll update this patch following yours suggestion, thanks.
^ permalink raw reply
* Re: [PATCH v1] OPP: Fix crashing when current OPP has unsupportable voltage
From: Dmitry Osipenko @ 2019-06-24 10:18 UTC (permalink / raw)
To: Viresh Kumar
Cc: Viresh Kumar, Nishanth Menon, Stephen Boyd, Marc Dietrich,
linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190624071857.6ji4zc55qugpqnij@vireshk-i7>
24.06.2019 10:18, Viresh Kumar пишет:
> On 23-06-19, 20:50, Dmitry Osipenko wrote:
>> Fix NULL dereference caused by a typo in the code. In particular it
>> happens when CPU is running on a frequency which has unsupportable voltage
>> (by regulator) defined in the OPP table and a custom set_opp() callback is
>> being used. The problem was spotted during of testing of upcoming update
>> for the NVIDIA Tegra CPUFreq driver.
>>
>> Cc: stable <stable@vger.kernel.org>
>> Fixes: 7e535993fa4f ("OPP: Separate out custom OPP handler specific code")
>> Reported-by: Marc Dietrich <marvin24@gmx.de>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>> drivers/opp/core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 9fda9a0ec016..89ec6aa220cf 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -685,7 +685,7 @@ static int _set_opp_custom(const struct opp_table *opp_table,
>>
>> data->old_opp.rate = old_freq;
>> size = sizeof(*old_supply) * opp_table->regulator_count;
>> - if (IS_ERR(old_supply))
>> + if (!old_supply)
>> memset(data->old_opp.supplies, 0, size);
>> else
>> memcpy(data->old_opp.supplies, old_supply, size);
>
> While the change is fine, the commit log isn't. It isn't about
> unsupportable voltage but frequency. The frequency the CPU is
> currently running at, is not present in the OPP table and so there is
> no corresponding OPP, hence no voltage supplies.
Ah, indeed! Looks like the reason for old OPP not being found was caused
by the appropriate OPP being disabled because of unsupportable voltage.
The offending higher "unsupportable" CPU freq was left after bootloader.
> I have applied this patch with following change log:
>
> commit 560d1bcad715c215e7ffe5d7cffe045974b623d0 (HEAD -> opp/linux-next)
> Author: Dmitry Osipenko <digetx@gmail.com>
> Date: Sun Jun 23 20:50:53 2019 +0300
>
> opp: Don't use IS_ERR on invalid supplies
>
> _set_opp_custom() receives a set of OPP supplies as its arguments and
> the caller of it passes NULL when the supplies are not valid. But
> _set_opp_custom(), by mistake, checks for error by performing
> IS_ERR(old_supply) on it which will always evaluate to false.
>
> The problem was spotted during of testing of upcoming update for the
> NVIDIA Tegra CPUFreq driver.
>
> Cc: stable <stable@vger.kernel.org>
> Fixes: 7e535993fa4f ("OPP: Separate out custom OPP handler specific code")
> Reported-by: Marc Dietrich <marvin24@gmx.de>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> [ Viresh: Massaged changelog ]
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/opp/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thank you very much!
^ permalink raw reply
* Re: [PATCH v1 0/3] Add required-opps support to devfreq passive gov
From: Viresh Kumar @ 2019-06-24 9:43 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, linux-kernel
In-Reply-To: <20190622003449.33707-1-saravanak@google.com>
On 21-06-19, 17:34, Saravana Kannan wrote:
> 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.
Can you please provide a real world example with DT code here so I
can understand it better ?
--
viresh
^ permalink raw reply
* Re: [PATCH 2/6] thermal/drivers/cpu_cooling: Unregister with the policy
From: Viresh Kumar @ 2019-06-24 9:39 UTC (permalink / raw)
To: Daniel Lezcano
Cc: edubezval, linux-kernel, Sudeep Holla, Rafael J. Wysocki,
Amit Daniel Kachhap, Javi Merino, Zhang Rui, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Keerthy,
open list:CPU FREQUENCY DRIVERS - ARM BIG LITTLE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:TI BANDGAP AND THERMAL DRIVER
In-Reply-To: <d31f65c8-53df-ae59-5f6f-211c0ddcff3f@linaro.org>
On 24-06-19, 09:45, Daniel Lezcano wrote:
> Actually I'm asking your opinion :)
>
> The structure in drivers/thermal/imx_thermal.c
>
> struct imx_thermal_data {
> struct cpufreq_policy *policy; <<<< in the thermal data ?!
> [ ... ]
> };
>
> And then:
>
> #ifdef CONFIG_CPU_FREQ
> /*
> * Create cooling device in case no #cooling-cells property is available in
> * CPU node
> */
> static int imx_thermal_register_legacy_cooling(struct imx_thermal_data
> *data)
> {
> struct device_node *np;
> int ret;
>
> data->policy = cpufreq_cpu_get(0);
> if (!data->policy) {
> pr_debug("%s: CPUFreq policy not found\n", __func__);
> return -EPROBE_DEFER;
> }
>
> np = of_get_cpu_node(data->policy->cpu, NULL);
>
> if (!np || !of_find_property(np, "#cooling-cells", NULL)) {
> data->cdev = cpufreq_cooling_register(data->policy);
> if (IS_ERR(data->cdev)) {
> ret = PTR_ERR(data->cdev);
> cpufreq_cpu_put(data->policy);
> return ret;
> }
> }
>
> return 0;
> }
>
> [ ... ]
>
> Shouldn't this be move in the drivers/cpufreq/<whatever driver> ?
Sure, we have platform specific drivers where this can be moved :)
--
viresh
^ permalink raw reply
* Re: [PATCH 1/6] cpufreq: Use existing stub functions instead of IS_ENABLED macro
From: Daniel Lezcano @ 2019-06-24 9:37 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, Viresh Kumar, Eduardo Valentin,
Linux Kernel Mailing List,
open list:CPU FREQUENCY SCALING FRAMEWORK
In-Reply-To: <2097869.93pjHihJNk@kreacher>
On 24/06/2019 11:30, Rafael J. Wysocki wrote:
> On Monday, June 24, 2019 11:22:19 AM CEST Daniel Lezcano wrote:
>> On 22/06/2019 11:12, Rafael J. Wysocki wrote:
>>> 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?
>>
>> I agree but I'm not a big fan of IS_ENABLED macros in the code when it
>> is possible to avoid them.
>>
>> What about adding a stub for that like:
>
> Well,
>
>> #ifdef CPU_THERMAL
>> static inline int cpufreq_is_cooling_dev(struct cpufreq_driver *drv)
>> {
>> return drv->flags & CPUFREQ_IS_COOLING_DEV;
>> }
>> #else
>> static inline int cpufreq_is_cooling_dev(struct cpufreq_driver *drv)
>> {
>> return 0;
>> }
>> #endif
>
> This may as well be defined as
>
> static inline int cpufreq_is_cooling_dev(struct cpufreq_driver *drv)
> {
> return IS_ENABLED(CPU_THERMAL) && drv->flags & CPUFREQ_IS_COOLING_DEV;
> }
>
> which is fewer lines of code.
Ah yes, even better.
> And I would call it something like cpufreq_thermal_control_enabled().
Ok, thanks!
--
<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 1/6] cpufreq: Use existing stub functions instead of IS_ENABLED macro
From: Rafael J. Wysocki @ 2019-06-24 9:30 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Rafael J. Wysocki, Viresh Kumar, Eduardo Valentin,
Linux Kernel Mailing List,
open list:CPU FREQUENCY SCALING FRAMEWORK
In-Reply-To: <b817a599-6564-b3d0-9c91-59c3fd5b5eb1@linaro.org>
On Monday, June 24, 2019 11:22:19 AM CEST Daniel Lezcano wrote:
> On 22/06/2019 11:12, Rafael J. Wysocki wrote:
> > 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?
>
> I agree but I'm not a big fan of IS_ENABLED macros in the code when it
> is possible to avoid them.
>
> What about adding a stub for that like:
Well,
> #ifdef CPU_THERMAL
> static inline int cpufreq_is_cooling_dev(struct cpufreq_driver *drv)
> {
> return drv->flags & CPUFREQ_IS_COOLING_DEV;
> }
> #else
> static inline int cpufreq_is_cooling_dev(struct cpufreq_driver *drv)
> {
> return 0;
> }
> #endif
This may as well be defined as
static inline int cpufreq_is_cooling_dev(struct cpufreq_driver *drv)
{
return IS_ENABLED(CPU_THERMAL) && drv->flags & CPUFREQ_IS_COOLING_DEV;
}
which is fewer lines of code.
And I would call it something like cpufreq_thermal_control_enabled().
^ permalink raw reply
* [PATCH 1/3] notifier: Fix broken error handling pattern
From: Peter Zijlstra @ 2019-06-24 9:18 UTC (permalink / raw)
To: Jessica Yu, linux-kernel, jpoimboe, jikos, mbenes, pmladek, ast,
daniel, akpm, peterz
Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Sam Protsenko,
Thomas Gleixner, Greg Kroah-Hartman, Alexios Zavras,
Allison Randal, Vasily Averin, Todd Brandt, linux-pm
In-Reply-To: <20190624091843.859714294@infradead.org>
The current notifiers have the following error handling pattern all
over the place:
int nr;
ret = __foo_notifier_call_chain(&chain, val_up, v, -1, &nr);
if (err & NOTIFIER_STOP_MASK)
__foo_notifier_call_chain(&chain, val_down, v, nr-1, NULL)
And aside from the endless repetition thereof, it is broken. Consider
blocking notifiers; both calls take and drop the rwsem, this means
that the notifier list can change in between the two calls, making @nr
meaningless.
Fix this by replacing all the __foo_notifier_call_chain() functions
with foo_notifier_call_chain_error() that embeds the above patter, but
ensures it is inside a single lock region.
XXX: It is probably still broken for the RCU (atomic, src) users
(cpu_pm_notifier).
Note: software_resume() error handling was broken afaict.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Sam Protsenko <semen.protsenko@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Alexios Zavras <alexios.zavras@intel.com>
Cc: Allison Randal <allison@lohutok.net>
Cc: Vasily Averin <vvs@virtuozzo.com>
Cc: Todd Brandt <todd.e.brandt@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/notifier.h | 17 ++-
kernel/cpu_pm.c | 51 +++++------
kernel/notifier.c | 159 +++++++++++++------------------------
kernel/power/hibernate.c | 26 ++----
kernel/power/main.c | 8 -
kernel/power/power.h | 3
kernel/power/suspend.c | 14 +--
kernel/power/user.c | 14 ---
tools/power/pm-graph/sleepgraph.py | 2
9 files changed, 118 insertions(+), 176 deletions(-)
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -165,20 +165,21 @@ extern int srcu_notifier_chain_unregiste
extern int atomic_notifier_call_chain(struct atomic_notifier_head *nh,
unsigned long val, void *v);
-extern int __atomic_notifier_call_chain(struct atomic_notifier_head *nh,
- unsigned long val, void *v, int nr_to_call, int *nr_calls);
extern int blocking_notifier_call_chain(struct blocking_notifier_head *nh,
unsigned long val, void *v);
-extern int __blocking_notifier_call_chain(struct blocking_notifier_head *nh,
- unsigned long val, void *v, int nr_to_call, int *nr_calls);
extern int raw_notifier_call_chain(struct raw_notifier_head *nh,
unsigned long val, void *v);
-extern int __raw_notifier_call_chain(struct raw_notifier_head *nh,
- unsigned long val, void *v, int nr_to_call, int *nr_calls);
extern int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
unsigned long val, void *v);
-extern int __srcu_notifier_call_chain(struct srcu_notifier_head *nh,
- unsigned long val, void *v, int nr_to_call, int *nr_calls);
+
+extern int atomic_notifier_call_chain_error(struct atomic_notifier_head *nh,
+ unsigned long val_up, unsigned long val_down, void *v);
+extern int blocking_notifier_call_chain_error(struct blocking_notifier_head *nh,
+ unsigned long val_up, unsigned long val_down, void *v);
+extern int raw_notifier_call_chain_error(struct raw_notifier_head *nh,
+ unsigned long val_up, unsigned long val_down, void *v);
+extern int srcu_notifier_call_chain_error(struct srcu_notifier_head *nh,
+ unsigned long val_up, unsigned long val_down, void *v);
#define NOTIFY_DONE 0x0000 /* Don't care */
#define NOTIFY_OK 0x0001 /* Suits me */
--- a/kernel/cpu_pm.c
+++ b/kernel/cpu_pm.c
@@ -15,7 +15,7 @@
static ATOMIC_NOTIFIER_HEAD(cpu_pm_notifier_chain);
-static int cpu_pm_notify(enum cpu_pm_event event, int nr_to_call, int *nr_calls)
+static int cpu_pm_notify(enum cpu_pm_event event)
{
int ret;
@@ -25,8 +25,23 @@ static int cpu_pm_notify(enum cpu_pm_eve
* RCU know this.
*/
rcu_irq_enter_irqson();
- ret = __atomic_notifier_call_chain(&cpu_pm_notifier_chain, event, NULL,
- nr_to_call, nr_calls);
+ ret = atomic_notifier_call_chain(&cpu_pm_notifier_chain, event, NULL);
+ rcu_irq_exit_irqson();
+
+ return notifier_to_errno(ret);
+}
+
+static int cpu_pm_notify_error(enum cpu_pm_event event_up, enum cpu_pm_event event_down)
+{
+ int ret;
+
+ /*
+ * __atomic_notifier_call_chain has a RCU read critical section, which
+ * could be disfunctional in cpu idle. Copy RCU_NONIDLE code to let
+ * RCU know this.
+ */
+ rcu_irq_enter_irqson();
+ ret = atomic_notifier_call_chain_error(&cpu_pm_notifier_chain, event_up, event_down, NULL);
rcu_irq_exit_irqson();
return notifier_to_errno(ret);
@@ -80,18 +95,7 @@ EXPORT_SYMBOL_GPL(cpu_pm_unregister_noti
*/
int cpu_pm_enter(void)
{
- int nr_calls;
- int ret = 0;
-
- ret = cpu_pm_notify(CPU_PM_ENTER, -1, &nr_calls);
- if (ret)
- /*
- * Inform listeners (nr_calls - 1) about failure of CPU PM
- * PM entry who are notified earlier to prepare for it.
- */
- cpu_pm_notify(CPU_PM_ENTER_FAILED, nr_calls - 1, NULL);
-
- return ret;
+ return cpu_pm_notify_error(CPU_PM_ENTER, CPU_PM_ENTER_FAILED);
}
EXPORT_SYMBOL_GPL(cpu_pm_enter);
@@ -109,7 +113,7 @@ EXPORT_SYMBOL_GPL(cpu_pm_enter);
*/
int cpu_pm_exit(void)
{
- return cpu_pm_notify(CPU_PM_EXIT, -1, NULL);
+ return cpu_pm_notify(CPU_PM_EXIT);
}
EXPORT_SYMBOL_GPL(cpu_pm_exit);
@@ -131,18 +135,7 @@ EXPORT_SYMBOL_GPL(cpu_pm_exit);
*/
int cpu_cluster_pm_enter(void)
{
- int nr_calls;
- int ret = 0;
-
- ret = cpu_pm_notify(CPU_CLUSTER_PM_ENTER, -1, &nr_calls);
- if (ret)
- /*
- * Inform listeners (nr_calls - 1) about failure of CPU cluster
- * PM entry who are notified earlier to prepare for it.
- */
- cpu_pm_notify(CPU_CLUSTER_PM_ENTER_FAILED, nr_calls - 1, NULL);
-
- return ret;
+ return cpu_pm_notify_enter(CPU_CLUSTER_PM_ENTER, CPU_CLUSTER_ENTER_FAILED);
}
EXPORT_SYMBOL_GPL(cpu_cluster_pm_enter);
@@ -163,7 +156,7 @@ EXPORT_SYMBOL_GPL(cpu_cluster_pm_enter);
*/
int cpu_cluster_pm_exit(void)
{
- return cpu_pm_notify(CPU_CLUSTER_PM_EXIT, -1, NULL);
+ return cpu_pm_notify(CPU_CLUSTER_PM_EXIT);
}
EXPORT_SYMBOL_GPL(cpu_cluster_pm_exit);
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -106,6 +106,19 @@ static int notifier_call_chain(struct no
}
NOKPROBE_SYMBOL(notifier_call_chain);
+static int notifier_call_chain_error(struct notifier_block **nl,
+ unsigned long val_up, unsigned long val_down,
+ void *v)
+{
+ int ret, nr = 0;
+
+ ret = notifier_call_chain(nl, val_up, v, -1, &nr);
+ if (ret & NOTIFY_STOP_MASK)
+ notifier_call_chain(nl, val_down, v, nr-1, NULL);
+
+ return ret;
+}
+
/*
* Atomic notifier chain routines. Registration and unregistration
* use a spinlock, and call_chain is synchronized by RCU (no locks).
@@ -156,43 +169,30 @@ int atomic_notifier_chain_unregister(str
}
EXPORT_SYMBOL_GPL(atomic_notifier_chain_unregister);
-/**
- * __atomic_notifier_call_chain - Call functions in an atomic notifier chain
- * @nh: Pointer to head of the atomic notifier chain
- * @val: Value passed unmodified to notifier function
- * @v: Pointer passed unmodified to notifier function
- * @nr_to_call: See the comment for notifier_call_chain.
- * @nr_calls: See the comment for notifier_call_chain.
- *
- * Calls each function in a notifier chain in turn. The functions
- * run in an atomic context, so they must not block.
- * This routine uses RCU to synchronize with changes to the chain.
- *
- * If the return value of the notifier can be and'ed
- * with %NOTIFY_STOP_MASK then atomic_notifier_call_chain()
- * will return immediately, with the return value of
- * the notifier function which halted execution.
- * Otherwise the return value is the return value
- * of the last notifier function called.
- */
-int __atomic_notifier_call_chain(struct atomic_notifier_head *nh,
- unsigned long val, void *v,
- int nr_to_call, int *nr_calls)
+int atomic_notifier_call_chain_error(struct atomic_notifier_head *nh,
+ unsigned long val_up, unsigned long val_down, void *v)
{
int ret;
rcu_read_lock();
- ret = notifier_call_chain(&nh->head, val, v, nr_to_call, nr_calls);
+ ret = notifier_call_chain_error(&nh->head, val_up, val_down, v);
rcu_read_unlock();
+
return ret;
}
-EXPORT_SYMBOL_GPL(__atomic_notifier_call_chain);
-NOKPROBE_SYMBOL(__atomic_notifier_call_chain);
+EXPORT_SYMBOL_GPL(atomic_notifier_call_chain_error);
+NOKPROBE_SYMBOL(atomic_notifier_call_chain_error);
int atomic_notifier_call_chain(struct atomic_notifier_head *nh,
unsigned long val, void *v)
{
- return __atomic_notifier_call_chain(nh, val, v, -1, NULL);
+ int ret;
+
+ rcu_read_lock();
+ ret = notifier_call_chain(&nh->head, val, v, -1, NULL);
+ rcu_read_unlock();
+
+ return ret;
}
EXPORT_SYMBOL_GPL(atomic_notifier_call_chain);
NOKPROBE_SYMBOL(atomic_notifier_call_chain);
@@ -285,27 +285,8 @@ int blocking_notifier_chain_unregister(s
}
EXPORT_SYMBOL_GPL(blocking_notifier_chain_unregister);
-/**
- * __blocking_notifier_call_chain - Call functions in a blocking notifier chain
- * @nh: Pointer to head of the blocking notifier chain
- * @val: Value passed unmodified to notifier function
- * @v: Pointer passed unmodified to notifier function
- * @nr_to_call: See comment for notifier_call_chain.
- * @nr_calls: See comment for notifier_call_chain.
- *
- * Calls each function in a notifier chain in turn. The functions
- * run in a process context, so they are allowed to block.
- *
- * If the return value of the notifier can be and'ed
- * with %NOTIFY_STOP_MASK then blocking_notifier_call_chain()
- * will return immediately, with the return value of
- * the notifier function which halted execution.
- * Otherwise the return value is the return value
- * of the last notifier function called.
- */
-int __blocking_notifier_call_chain(struct blocking_notifier_head *nh,
- unsigned long val, void *v,
- int nr_to_call, int *nr_calls)
+int blocking_notifier_call_chain_error(struct blocking_notifier_head *nh,
+ unsigned long val_up, unsigned long val_down, void *v)
{
int ret = NOTIFY_DONE;
@@ -316,18 +297,29 @@ int __blocking_notifier_call_chain(struc
*/
if (rcu_access_pointer(nh->head)) {
down_read(&nh->rwsem);
- ret = notifier_call_chain(&nh->head, val, v, nr_to_call,
- nr_calls);
+ ret = notifier_call_chain_error(&nh->head, val_up, val_down, v);
up_read(&nh->rwsem);
}
return ret;
}
-EXPORT_SYMBOL_GPL(__blocking_notifier_call_chain);
+EXPORT_SYMBOL_GPL(blocking_notifier_call_chain_error);
int blocking_notifier_call_chain(struct blocking_notifier_head *nh,
unsigned long val, void *v)
{
- return __blocking_notifier_call_chain(nh, val, v, -1, NULL);
+ int ret = NOTIFY_DONE;
+
+ /*
+ * We check the head outside the lock, but if this access is
+ * racy then it does not matter what the result of the test
+ * is, we re-check the list after having taken the lock anyway:
+ */
+ if (rcu_access_pointer(nh->head)) {
+ down_read(&nh->rwsem);
+ ret = notifier_call_chain(&nh->head, val, v, -1, NULL);
+ up_read(&nh->rwsem);
+ }
+ return ret;
}
EXPORT_SYMBOL_GPL(blocking_notifier_call_chain);
@@ -370,37 +362,17 @@ int raw_notifier_chain_unregister(struct
}
EXPORT_SYMBOL_GPL(raw_notifier_chain_unregister);
-/**
- * __raw_notifier_call_chain - Call functions in a raw notifier chain
- * @nh: Pointer to head of the raw notifier chain
- * @val: Value passed unmodified to notifier function
- * @v: Pointer passed unmodified to notifier function
- * @nr_to_call: See comment for notifier_call_chain.
- * @nr_calls: See comment for notifier_call_chain
- *
- * Calls each function in a notifier chain in turn. The functions
- * run in an undefined context.
- * All locking must be provided by the caller.
- *
- * If the return value of the notifier can be and'ed
- * with %NOTIFY_STOP_MASK then raw_notifier_call_chain()
- * will return immediately, with the return value of
- * the notifier function which halted execution.
- * Otherwise the return value is the return value
- * of the last notifier function called.
- */
-int __raw_notifier_call_chain(struct raw_notifier_head *nh,
- unsigned long val, void *v,
- int nr_to_call, int *nr_calls)
+int raw_notifier_call_chain_error(struct raw_notifier_head *nh,
+ unsigned long val_up, unsigned long val_down, void *v)
{
- return notifier_call_chain(&nh->head, val, v, nr_to_call, nr_calls);
+ return notifier_call_chain_error(&nh->head, val_up, val_down, v);
}
-EXPORT_SYMBOL_GPL(__raw_notifier_call_chain);
+EXPORT_SYMBOL_GPL(raw_notifier_call_chain_error);
int raw_notifier_call_chain(struct raw_notifier_head *nh,
unsigned long val, void *v)
{
- return __raw_notifier_call_chain(nh, val, v, -1, NULL);
+ return notifier_call_chain(&nh->head, val, v, -1, NULL);
}
EXPORT_SYMBOL_GPL(raw_notifier_call_chain);
@@ -471,42 +443,29 @@ int srcu_notifier_chain_unregister(struc
}
EXPORT_SYMBOL_GPL(srcu_notifier_chain_unregister);
-/**
- * __srcu_notifier_call_chain - Call functions in an SRCU notifier chain
- * @nh: Pointer to head of the SRCU notifier chain
- * @val: Value passed unmodified to notifier function
- * @v: Pointer passed unmodified to notifier function
- * @nr_to_call: See comment for notifier_call_chain.
- * @nr_calls: See comment for notifier_call_chain
- *
- * Calls each function in a notifier chain in turn. The functions
- * run in a process context, so they are allowed to block.
- *
- * If the return value of the notifier can be and'ed
- * with %NOTIFY_STOP_MASK then srcu_notifier_call_chain()
- * will return immediately, with the return value of
- * the notifier function which halted execution.
- * Otherwise the return value is the return value
- * of the last notifier function called.
- */
-int __srcu_notifier_call_chain(struct srcu_notifier_head *nh,
- unsigned long val, void *v,
- int nr_to_call, int *nr_calls)
+int srcu_notifier_call_chain_error(struct srcu_notifier_head *nh,
+ unsigned long val_up, unsigned long val_down, void *v)
{
int ret;
int idx;
idx = srcu_read_lock(&nh->srcu);
- ret = notifier_call_chain(&nh->head, val, v, nr_to_call, nr_calls);
+ ret = notifier_call_chain_error(&nh->head, val_up, val_down, v);
srcu_read_unlock(&nh->srcu, idx);
return ret;
}
-EXPORT_SYMBOL_GPL(__srcu_notifier_call_chain);
+EXPORT_SYMBOL_GPL(srcu_notifier_call_chain_error);
int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
unsigned long val, void *v)
{
- return __srcu_notifier_call_chain(nh, val, v, -1, NULL);
+ int ret;
+ int idx;
+
+ idx = srcu_read_lock(&nh->srcu);
+ ret = notifier_call_chain(&nh->head, val, v, -1, NULL);
+ srcu_read_unlock(&nh->srcu, idx);
+ return ret;
}
EXPORT_SYMBOL_GPL(srcu_notifier_call_chain);
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -693,8 +693,8 @@ static int load_image_and_restore(void)
*/
int hibernate(void)
{
- int error, nr_calls = 0;
bool snapshot_test = false;
+ int error;
if (!hibernation_available()) {
pm_pr_dbg("Hibernation not available.\n");
@@ -710,11 +710,9 @@ int hibernate(void)
pr_info("hibernation entry\n");
pm_prepare_console();
- error = __pm_notifier_call_chain(PM_HIBERNATION_PREPARE, -1, &nr_calls);
- if (error) {
- nr_calls--;
- goto Exit;
- }
+ error = pm_notifier_call_chain_error(PM_HIBERNATION_PREPARE, PM_POST_HIBERNATION);
+ if (error)
+ goto Restore;
ksys_sync_helper();
@@ -772,7 +770,8 @@ int hibernate(void)
/* Don't bother checking whether freezer_test_done is true */
freezer_test_done = false;
Exit:
- __pm_notifier_call_chain(PM_POST_HIBERNATION, nr_calls, NULL);
+ pm_notifier_call_chain(PM_POST_HIBERNATION);
+ Restore:
pm_restore_console();
atomic_inc(&snapshot_device_available);
Unlock:
@@ -800,7 +799,7 @@ int hibernate(void)
*/
static int software_resume(void)
{
- int error, nr_calls = 0;
+ int error;
/*
* If the user said "noresume".. bail out early.
@@ -887,11 +886,9 @@ static int software_resume(void)
pr_info("resume from hibernation\n");
pm_prepare_console();
- error = __pm_notifier_call_chain(PM_RESTORE_PREPARE, -1, &nr_calls);
- if (error) {
- nr_calls--;
- goto Close_Finish;
- }
+ error = pm_notifier_call_chain_error(PM_RESTORE_PREPARE, PM_POST_RESTORE);
+ if (error)
+ goto Restore;
pm_pr_dbg("Preparing processes for restore.\n");
error = freeze_processes();
@@ -900,7 +897,8 @@ static int software_resume(void)
error = load_image_and_restore();
thaw_processes();
Finish:
- __pm_notifier_call_chain(PM_POST_RESTORE, nr_calls, NULL);
+ pm_notifier_call_chain(PM_POST_RESTORE);
+ Restore:
pm_restore_console();
pr_info("resume from hibernation failed (%d)\n", error);
atomic_inc(&snapshot_device_available);
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -79,18 +79,18 @@ int unregister_pm_notifier(struct notifi
}
EXPORT_SYMBOL_GPL(unregister_pm_notifier);
-int __pm_notifier_call_chain(unsigned long val, int nr_to_call, int *nr_calls)
+int pm_notifier_call_chain_error(unsigned long val_up, unsigned long val_down)
{
int ret;
- ret = __blocking_notifier_call_chain(&pm_chain_head, val, NULL,
- nr_to_call, nr_calls);
+ ret = blocking_notifier_call_chain_error(&pm_chain_head, val_up, val_down, NULL);
return notifier_to_errno(ret);
}
+
int pm_notifier_call_chain(unsigned long val)
{
- return __pm_notifier_call_chain(val, -1, NULL);
+ return blocking_notifier_call_chain(&pm_chain_head, val, NULL);
}
/* If set, devices may be suspended and resumed asynchronously. */
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -212,8 +212,7 @@ static inline void suspend_test_finish(c
#ifdef CONFIG_PM_SLEEP
/* kernel/power/main.c */
-extern int __pm_notifier_call_chain(unsigned long val, int nr_to_call,
- int *nr_calls);
+extern int pm_notifier_call_chain_error(unsigned long val_up, unsigned long val_down);
extern int pm_notifier_call_chain(unsigned long val);
#endif
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -352,18 +352,16 @@ static int suspend_test(int level)
*/
static int suspend_prepare(suspend_state_t state)
{
- int error, nr_calls = 0;
+ int error;
if (!sleep_state_supported(state))
return -EPERM;
pm_prepare_console();
- error = __pm_notifier_call_chain(PM_SUSPEND_PREPARE, -1, &nr_calls);
- if (error) {
- nr_calls--;
- goto Finish;
- }
+ error = pm_notifier_call_chain_error(PM_SUSPEND_PREPARE, PM_POST_SUSPEND);
+ if (error)
+ goto Restore;
trace_suspend_resume(TPS("freeze_processes"), 0, true);
error = suspend_freeze_processes();
@@ -373,8 +371,8 @@ static int suspend_prepare(suspend_state
suspend_stats.failed_freeze++;
dpm_save_failed_step(SUSPEND_FREEZE);
- Finish:
- __pm_notifier_call_chain(PM_POST_SUSPEND, nr_calls, NULL);
+ pm_notifier_call_chain(PM_POST_SUSPEND);
+ Restore:
pm_restore_console();
return error;
}
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -44,7 +44,7 @@ atomic_t snapshot_device_available = ATO
static int snapshot_open(struct inode *inode, struct file *filp)
{
struct snapshot_data *data;
- int error, nr_calls = 0;
+ int error;
if (!hibernation_available())
return -EPERM;
@@ -71,9 +71,7 @@ static int snapshot_open(struct inode *i
swap_type_of(swsusp_resume_device, 0, NULL) : -1;
data->mode = O_RDONLY;
data->free_bitmaps = false;
- error = __pm_notifier_call_chain(PM_HIBERNATION_PREPARE, -1, &nr_calls);
- if (error)
- __pm_notifier_call_chain(PM_POST_HIBERNATION, --nr_calls, NULL);
+ error = pm_notifier_call_chain_error(PM_HIBERNATION_PREPARE, PM_POST_HIBERNATION);
} else {
/*
* Resuming. We may need to wait for the image device to
@@ -83,15 +81,11 @@ static int snapshot_open(struct inode *i
data->swap = -1;
data->mode = O_WRONLY;
- error = __pm_notifier_call_chain(PM_RESTORE_PREPARE, -1, &nr_calls);
+ error = pm_notifier_call_chain_error(PM_RESTORE_PREPARE, PM_POST_RESTORE);
if (!error) {
error = create_basic_memory_bitmaps();
data->free_bitmaps = !error;
- } else
- nr_calls--;
-
- if (error)
- __pm_notifier_call_chain(PM_POST_RESTORE, nr_calls, NULL);
+ }
}
if (error)
atomic_inc(&snapshot_device_available);
--- a/tools/power/pm-graph/sleepgraph.py
+++ b/tools/power/pm-graph/sleepgraph.py
@@ -146,7 +146,7 @@ from subprocess import call, Popen, PIPE
tracefuncs = {
'sys_sync': {},
'ksys_sync': {},
- '__pm_notifier_call_chain': {},
+ 'pm_notifier_call_chain_error': {},
'pm_prepare_console': {},
'pm_notifier_call_chain': {},
'freeze_processes': {},
^ permalink raw reply
* Re: [PATCH 1/6] cpufreq: Use existing stub functions instead of IS_ENABLED macro
From: Daniel Lezcano @ 2019-06-24 9:22 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Viresh Kumar, Eduardo Valentin, Linux Kernel Mailing List,
Rafael J. Wysocki, open list:CPU FREQUENCY SCALING FRAMEWORK
In-Reply-To: <CAJZ5v0j0q+Z+FRpVuj39ML_c5ijo-veMMMSANdoDz1ZxAK3RgQ@mail.gmail.com>
On 22/06/2019 11:12, Rafael J. Wysocki wrote:
> 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?
I agree but I'm not a big fan of IS_ENABLED macros in the code when it
is possible to avoid them.
What about adding a stub for that like:
#ifdef CPU_THERMAL
static inline int cpufreq_is_cooling_dev(struct cpufreq_driver *drv)
{
return drv->flags & CPUFREQ_IS_COOLING_DEV;
}
#else
static inline int cpufreq_is_cooling_dev(struct cpufreq_driver *drv)
{
return 0;
}
#endif
?
>> 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
>>
--
<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 1/6] cpufreq: Use existing stub functions instead of IS_ENABLED macro
From: Viresh Kumar @ 2019-06-24 9:08 UTC (permalink / raw)
To: Daniel Lezcano
Cc: edubezval, linux-kernel, Rafael J. Wysocki,
open list:CPU FREQUENCY SCALING FRAMEWORK
In-Reply-To: <0ce0d1ca-154d-fca3-f739-573ecbd2b0db@linaro.org>
On 24-06-19, 10:53, Daniel Lezcano wrote:
>
> Hi Viresh,
>
> On 21/06/2019 15:22, Daniel Lezcano 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.
> >
> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
> what about this one?
I didn't say something because Rafael already pointed out something
which he wanted. Actually he was the one who suggested it to Amit
initially.
http://lore.kernel.org/lkml/CAJZ5v0huESFqOADqyym-ci-XcWpL4tbcd9a-jxe_KArXKpHFyw@mail.gmail.com
--
viresh
^ permalink raw reply
* Re: [PATCH 1/6] cpufreq: Use existing stub functions instead of IS_ENABLED macro
From: Daniel Lezcano @ 2019-06-24 9:07 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Viresh Kumar, Eduardo Valentin, Linux Kernel Mailing List,
Rafael J. Wysocki, open list:CPU FREQUENCY SCALING FRAMEWORK
In-Reply-To: <CAJZ5v0gR1O=21FRXrDvCi_wingO5W74w9PX8s5s2=1aesYo4yg@mail.gmail.com>
Hi Rafael,
On 24/06/2019 11:00, Rafael J. Wysocki wrote:
> On Mon, Jun 24, 2019 at 10:53 AM Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>>
>>
>> Hi Viresh,
>>
>> On 21/06/2019 15:22, Daniel Lezcano 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.
>>>
>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>
>> what about this one?
>
> Care to respond to my question regarding this one in the first place, please?
Ah yes, sorry, I missed your email.
--
<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 1/6] cpufreq: Use existing stub functions instead of IS_ENABLED macro
From: Rafael J. Wysocki @ 2019-06-24 9:00 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: <0ce0d1ca-154d-fca3-f739-573ecbd2b0db@linaro.org>
On Mon, Jun 24, 2019 at 10:53 AM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
>
> Hi Viresh,
>
> On 21/06/2019 15:22, Daniel Lezcano 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.
> >
> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
> what about this one?
Care to respond to my question regarding this one in the first place, please?
^ permalink raw reply
* Re: [PATCH 1/6] cpufreq: Use existing stub functions instead of IS_ENABLED macro
From: Daniel Lezcano @ 2019-06-24 8:53 UTC (permalink / raw)
To: viresh.kumar
Cc: edubezval, linux-kernel, Rafael J. Wysocki,
open list:CPU FREQUENCY SCALING FRAMEWORK
In-Reply-To: <20190621132302.30414-1-daniel.lezcano@linaro.org>
Hi Viresh,
On 21/06/2019 15:22, Daniel Lezcano 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.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
what about this one?
> ---
> 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;
> }
>
--
<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 v4 10/11] dt-bindings: thermal: add binding document for h3 thermal controller
From: Maxime Ripard @ 2019-06-24 8:50 UTC (permalink / raw)
To: Yangtao Li
Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland, wens,
davem, gregkh, mchehab+samsung, linus.walleij, nicolas.ferre,
paulmck, linux-pm, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20190623164206.7467-11-tiny.windzz@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2401 bytes --]
On Sun, Jun 23, 2019 at 12:42:05PM -0400, Yangtao Li wrote:
> This patch adds binding document for allwinner h3 thermal controller.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
> .../bindings/thermal/sun8i-thermal.yaml | 29 +++++++++++++++++--
> 1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml b/Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml
> index 2c5acc61ed03..1eaf68b5dd5a 100644
> --- a/Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml
> @@ -16,6 +16,7 @@ description: |-
> properties:
> compatible:
> enum:
> + - allwinner,sun8i-h3-ths
> - allwinner,sun50i-h6-ths
>
> reg:
> @@ -29,13 +30,22 @@ properties:
>
> clocks:
> minItems: 1
> - maxItems: 1
> + maxItems: 2
>
> clock-names:
> - const: bus
> + items:
> + - const: bus
> + - const: ahb
You need a min/maxItems here as well. Otherwise, on the H6, you will
have a warning since you asked for an array of two items, bus and ahb,
while you only have a single one.
> '#thermal-sensor-cells':
> - const: 1
> + enum: [ 0, 1 ]
> + description: |
> + Definition depends on soc version:
> +
> + For "allwinner,sun8i-h3-ths",
> + value must be 0.
> + For "allwinner,sun50i-h6-ths",
> + value must be 1.
This must be checked using a conditional.
Something like:
if:
properties:
compatible:
const: allwinner,sun8i-h3-ths
then:
properties:
"#thermal-sensor-cells":
const: 0
else:
properties:
"#thermal-sensor-cells":
const: 1
> nvmem-cells:
> items:
> @@ -55,6 +65,19 @@ required:
> - '#thermal-sensor-cells'
>
> examples:
> + - |
> + ths: ths@1c25000 {
> + compatible = "allwinner,sun8i-h3-ths";
> + reg = <0x01c25000 0x400>;
> + clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
> + clock-names = "bus", "ahb";
> + resets = <&ccu RST_BUS_THS>;
> + interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
> + nvmem-cells = <&tsen_calib>;
> + nvmem-cell-names = "calib";
> + #thermal-sensor-cells = <0>;
> + };
Same remark here, it won't compile
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v4 02/11] dt-bindings: thermal: add binding document for h6 thermal controller
From: Maxime Ripard @ 2019-06-24 8:47 UTC (permalink / raw)
To: Yangtao Li
Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland, wens,
davem, gregkh, mchehab+samsung, linus.walleij, nicolas.ferre,
paulmck, linux-pm, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20190623164206.7467-3-tiny.windzz@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2324 bytes --]
Hi,
On Sun, Jun 23, 2019 at 12:41:57PM -0400, Yangtao Li wrote:
> This patch adds binding document for allwinner h6 thermal controller.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> ---
> .../bindings/thermal/sun8i-thermal.yaml | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml
>
> diff --git a/Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml b/Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml
> new file mode 100644
> index 000000000000..2c5acc61ed03
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/sun8i-thermal.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/sun8i-thermal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner SUN8I Thermal Controller Device Tree Bindings
> +
> +maintainers:
> + - Yangtao Li <tiny.windzz@gmail.com>
> +
> +description: |-
> + This describes the device tree binding for the Allwinner thermal
> + controller which measures the on-SoC temperatures.
> +
> +properties:
> + compatible:
> + enum:
> + - allwinner,sun50i-h6-ths
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> +
> + clocks:
> + minItems: 1
> + maxItems: 1
You can drop the minItems there
> + nvmem-cells:
> + items:
> + - description: ths calibrate data
> +
> + nvmem-cell-names:
> + items:
> + - const: calib
And for these two, you don't need the items either, it can be directly
const: calib (and the description for the first one).
> +required:
> + - compatible
> + - reg
> + - reset
> + - clocks
> + - clock-names
> + - interrupts
> + - '#thermal-sensor-cells'
> +
> +examples:
> + - |
> + ths: ths@5070400 {
> + compatible = "allwinner,sun50i-h6-ths";
> + reg = <0x05070400 0x100>;
> + clocks = <&ccu CLK_BUS_THS>;
> + clock-names = "bus";
> + resets = <&ccu RST_BUS_THS>;
> + interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
Did you try to run make dtbs_check? That one will probably not
compile.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* RE: [PATCH V15 1/5] dt-bindings: fsl: scu: add thermal binding
From: Anson Huang @ 2019-06-24 8:04 UTC (permalink / raw)
To: Anson Huang, robh+dt@kernel.org, mark.rutland@arm.com,
corbet@lwn.net, shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
catalin.marinas@arm.com, will.deacon@arm.com, rui.zhang@intel.com,
edubezval@gmail.com, daniel.lezcano@linaro.org, Aisheng Dong,
ulf.hansson@linaro.org, Peng Fan, mchehab+samsung@kernel.org,
linux@roeck-us.net, Daniel Baluta, maxime.ripard@bootlin.com,
horms+renesas@verge.net.au, olof@lixom.net,
jagan@amarulasolutions.com, bjorn.andersson@linaro.org,
Leonard Crestez, dinguyen@kernel.org,
enric.balletbo@collabora.com, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <20190618021820.14885-1-Anson.Huang@nxp.com>
Hi, Daniel/Rui/Eduardo
Could you please take a look at this patch series?
Anson
> From: Anson Huang <Anson.Huang@nxp.com>
>
> NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as system
> controller, the system controller is in charge of system power, clock and
> thermal sensors etc. management, Linux kernel has to communicate with
> system controller via MU (message unit) IPC to get temperature from thermal
> sensors, this patch adds binding doc for i.MX system controller thermal
> driver.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
> No change.
> ---
> .../devicetree/bindings/arm/freescale/fsl,scu.txt | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> index a575e42..fc3844e 100644
> --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> @@ -155,6 +155,17 @@ Required properties:
> Optional properties:
> - timeout-sec: contains the watchdog timeout in seconds.
>
> +Thermal bindings based on SCU Message Protocol
> +------------------------------------------------------------
> +
> +Required properties:
> +- compatible: Should be :
> + "fsl,imx8qxp-sc-thermal"
> + followed by "fsl,imx-sc-thermal";
> +
> +- #thermal-sensor-cells: See
> Documentation/devicetree/bindings/thermal/thermal.txt
> + for a description.
> +
> Example (imx8qxp):
> -------------
> aliases {
> @@ -222,6 +233,11 @@ firmware {
> compatible = "fsl,imx8qxp-sc-wdt", "fsl,imx-sc-wdt";
> timeout-sec = <60>;
> };
> +
> + tsens: thermal-sensor {
> + compatible = "fsl,imx8qxp-sc-thermal", "fsl,imx-sc-
> thermal";
> + #thermal-sensor-cells = <1>;
> + };
> };
> };
>
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH 2/6] thermal/drivers/cpu_cooling: Unregister with the policy
From: Daniel Lezcano @ 2019-06-24 7:45 UTC (permalink / raw)
To: Viresh Kumar
Cc: edubezval, linux-kernel, Sudeep Holla, Rafael J. Wysocki,
Amit Daniel Kachhap, Javi Merino, Zhang Rui, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Keerthy,
open list:CPU FREQUENCY DRIVERS - ARM BIG LITTLE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:TI BANDGAP AND THERMAL DRIVER
In-Reply-To: <20190624073747.hf7jd6ulkmebbxtm@vireshk-i7>
On 24/06/2019 09:37, Viresh Kumar wrote:
> On 24-06-19, 09:30, Daniel Lezcano wrote:
>> On 24/06/2019 08:03, Viresh Kumar wrote:
>>> On 21-06-19, 15:22, Daniel Lezcano wrote:
>>>> Currently the function cpufreq_cooling_register() returns a cooling
>>>> device pointer which is used back as a pointer to call the function
>>>> cpufreq_cooling_unregister(). Even if it is correct, it would make
>>>> sense to not leak the structure inside a cpufreq driver and keep the
>>>> code thermal code self-encapsulate. Moreover, that forces to add an
>>>> extra variable in each driver using this function.
>>>>
>>>> Instead of passing the cooling device to unregister, pass the policy.
>>>>
>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> ---
>>>> drivers/cpufreq/arm_big_little.c | 2 +-
>>>> drivers/cpufreq/cpufreq.c | 2 +-
>>>> drivers/thermal/cpu_cooling.c | 18 ++++++++++--------
>>>> drivers/thermal/imx_thermal.c | 4 ++--
>>>> .../thermal/ti-soc-thermal/ti-thermal-common.c | 2 +-
>>>> include/linux/cpu_cooling.h | 6 +++---
>>>> 6 files changed, 18 insertions(+), 16 deletions(-)
>>>
>>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>>
>> Just a side note, does it make sense to have the function called from
>> imx_thermal.c and ti-thermal-common.c? Sounds like also a leakage from
>> cpufreq to thermal drivers, no?
>
> I am not sure what you are proposing here :)
Actually I'm asking your opinion :)
The structure in drivers/thermal/imx_thermal.c
struct imx_thermal_data {
struct cpufreq_policy *policy; <<<< in the thermal data ?!
[ ... ]
};
And then:
#ifdef CONFIG_CPU_FREQ
/*
* Create cooling device in case no #cooling-cells property is available in
* CPU node
*/
static int imx_thermal_register_legacy_cooling(struct imx_thermal_data
*data)
{
struct device_node *np;
int ret;
data->policy = cpufreq_cpu_get(0);
if (!data->policy) {
pr_debug("%s: CPUFreq policy not found\n", __func__);
return -EPROBE_DEFER;
}
np = of_get_cpu_node(data->policy->cpu, NULL);
if (!np || !of_find_property(np, "#cooling-cells", NULL)) {
data->cdev = cpufreq_cooling_register(data->policy);
if (IS_ERR(data->cdev)) {
ret = PTR_ERR(data->cdev);
cpufreq_cpu_put(data->policy);
return ret;
}
}
return 0;
}
[ ... ]
Shouldn't this be move in the drivers/cpufreq/<whatever driver> ?
--
<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
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