Install the callbacks via the state machine and let the core invoke
the callbacks on the already online CPUs.
Replace the wrmsr/rdmrs_on_cpu() calls in the hotplug callbacks as they are
guaranteed to be invoked on the incoming/outgoing cpu.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Packages are kept in a list, which must be searched over and over.
We can be smarter than that and just store the package pointers in an array
which is allocated at init time. Sizing of the array is determined from the
topology information. That makes the package search a simple array lookup.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Delayed work structs are held in a static percpu storage, which makes no
sense at all because work is strictly per package and we never schedule
more than one work per package.
Aside of that the work cancelation in the hotplug is broken when the work
is queued on the outgoing cpu and canceled. Nothing reschedules the work on
another online cpu in the package, so the interrupts stay disabled and the
work_scheduled flag stays active.
Move the delayed work struct into the package struct, which is the only
sensible place to have it.
To simplify the cancelation logic schedule the work always on the cpu which
is the target for the sysfs files. This is required so the cancelation
logic in the cpu offline path cancels only when the outgoing cpu is the
current target and reschedule the work when there is still a online
CPU in the package.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Storage for a boolean information whether work is scheduled for a package
is kept in separate allocated storage, which is resized when the number of
detected packages grows.
With the proper locking in place this is a completely pointless exercise
because we can simply stick it into the per package struct.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
The work cancellation code, the thermal zone unregistering, the work code
and the interrupt notification function are racy against each other and
against cpu hotplug and module exit. The random locking sprinkeled all
over the place does not help anything and probably exists to make people
feel good. The resulting issues (mainly use after free) are probably
hard to trigger, but they clearly exist
Protect the package list with a spinlock so it can be accessed from the
interrupt notifier and also from the work function. The add/removal code in
the hotplug callbacks take the lock for list manipulation. That makes sure
that on removal neither the interrupt notifier nor the work function can
access the about to be freed package structure anymore.
The thermal zone unregistering is another trainwreck. It's not serialized
against the work function. So unregistering the zone device can race with
the work function and cause havoc.
Protect the thermal zone with a mutex, which is held in the work
function to make sure that the zone device is not being unregistered
concurrently.
To solve the module exit issues, we simply invoke the cpu offline callback
and let it work its magic. For that it's required to keep track of the
participating cpus in a package, because topology_core_mask is not affected
by calling the offline callback for teardown of the driver, so it would
never free the package as there is always a valid target in
topology_core_mask.
Use proper names for the locks so it's clear what they are for and add a
pile of comments to explain the protection rules.
It's amazing that fixing the locking and adding 30 lines of comments
explaining it still removes more lines than it adds.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Coding style fixups and replacement of overly complex constructs and random
error codes instead of returning the real ones. This mess makes the eyes bleeding.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Any randomly chosen struct name is more descriptive than phy_dev_entry.
Rename the whole thing to struct pkg_device, which describes the content
reasonably well and use the same variable name throughout the code so it
gets readable. Rename the msr struct members as well.
No functional change.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
There is no point in the whole package data refcounting dance because
topology_core_cpumask tells us whether this is the last cpu in the
package. If yes, then the package can go, if not it stays. It's already
serialized via the hotplug code.
While at it rename the first_cpu member of the package structure to
cpu. The first has absolutely no meaning.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
The threshold callbacks are installed before the initialization of the
online cpus has succeeded and removed after the teardown has been
done. That's both wrong as callbacks might be invoked into a half
initialized or torn down state.
Move them to the proper places: Last in init() and first in exit().
While at it shorten the insane long and horrible named function names.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
find_next_sibling() iterates over the online cpus and searches for a cpu
with the same package id as the current cpu. This is a pointless exercise
as topology_core_cpumask() allows a simple cpumask search for an online cpu
on the same package.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
In pkg_temp_thermal_device_remove() the package device is searched at the
beginning of the function. When the device refcount becomes zero another
search for the same device is conducted. Remove the pointless loop and use
the device pointer which was retrieved at the beginning of the function.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Wenn a package is removed nothing restores the thermal interrupt MSR so
the content will be stale when a CPU of that package becomes online again.
Aside of that the work function reenables interrupts before acknowledging
the current one, which is the wrong order to begin with.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
In the critical sysfs entry the thermal hwmon was returning wrong
temperature to the user-space. It was reporting the temperature of the
first trip point instead of the temperature of critical trip point.
For example:
/sys/class/hwmon/hwmon0/temp1_crit:50000
/sys/class/thermal/thermal_zone0/trip_point_0_temp:50000
/sys/class/thermal/thermal_zone0/trip_point_0_type:active
/sys/class/thermal/thermal_zone0/trip_point_3_temp:120000
/sys/class/thermal/thermal_zone0/trip_point_3_type:critical
Since commit e68b16abd9 ("thermal: add hwmon sysfs I/F") the driver
have been registering a sysfs entry if get_crit_temp() callback was
provided. However when accessed, it was calling get_trip_temp() instead
of the get_crit_temp().
Fixes: e68b16abd9 ("thermal: add hwmon sysfs I/F")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
When the devfreq cooling device was designed, it was an oversight not to
pass a pointer to the struct devfreq as the first parameters of the
callbacks. The design patterns of the kernel suggest it for a good
reason.
By passing a pointer to struct devfreq, the driver can register one
function that works with multiple devices. With the current
implementation, a driver that can work with multiple devices has to
create multiple copies of the same function with different parameters so
that each devfreq_cooling_device can use the appropriate one. By
passing a pointer to struct devfreq, the driver can identify which
device it's referring to.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>
Reviewed-by: Ørjan Eide <orjan.eide@arm.com>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Currently the protection #ifdef CONFIG_DEVFREQ_THERMAL cuts the needed
structures devfreq_cooling_ops and devfreq_cooling_device.
The functions which are supposed to provide the empty implementation complain
about unknown structures.
Similar solution is present in include/linux/devfreq.h.
Reviewed-by: Ørjan Eide <orjan.eide@arm.com>
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
"rockchip,hw-tshut-temp", "rockchip,hw-tshut-mode" and
"rockchip,hw-tshut-polarity" are not a required properties
actually as the code could also work by loading the default
settings there. So it is apprently misleading, although we
prefer to get these from DT. And it seems we miss the 'rockchip,grf'
here which should also be an optional property.
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
It is no necessary to print warning agian and again if we don't
add rockchip,grf for dt, otherwise I saw the following log when
doing suspend-2-resume. We only need to print it once when parsing
dt. It looks quite trivial but the log is apparently verbose.
[ 26.615415] PM: early resume of devices complete after 1.539 msecs
[ 26.622002] rk_tsadcv2_initialize: Missing rockchip,grf property
[ 26.629359] rk_gmac-dwmac ff290000.ethernet: init for RGMII
[ 26.639794] PM: resume of devices complete after 18.109 msecs
[ 26.646925] Restarting tasks ... done.
Reviewed-by: Caesar Wang <wxt@rock-chips.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
If the driver is built as a module, autoload won't work because the module
alias information is not filled. So user-space can't match the registered
device with the corresponding module.
Export the module alias information using the MODULE_DEVICE_TABLE() macro.
Before this patch:
$ modinfo drivers/thermal/db8500_thermal.ko | grep alias
$
After this patch:
$ modinfo drivers/thermal/db8500_thermal.ko | grep alias
alias: of:N*T*Cstericsson,db8500-thermalC*
alias: of:N*T*Cstericsson,db8500-thermal
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
If the driver is built as a module, autoload won't work because the module
alias information is not filled. So user-space can't match the registered
device with the corresponding module.
Export the module alias information using the MODULE_DEVICE_TABLE() macro.
Before this patch:
$ modinfo drivers/thermal/tango_thermal.ko | grep alias
$
After this patch:
$ modinfo drivers/thermal/tango_thermal.ko | grep alias
alias: of:N*T*Csigma,smp8758-thermalC*
alias: of:N*T*Csigma,smp8758-thermal
Acked-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
If the driver is built as a module, autoload won't work because the module
alias information is not filled. So user-space can't match the registered
device with the corresponding module.
Export the module alias information using the MODULE_DEVICE_TABLE() macro.
Before this patch:
$ modinfo drivers/thermal/max77620_thermal.ko | grep alias
$
After this patch:
$ modinfo drivers/thermal/max77620_thermal.ko | grep alias
alias: platform:max77620-thermal
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
We only read the first element of the reg property to figure out
the offset of the temperature sensor inside the PMIC.
Furthermore, we want to remove the second element in DT, so just
don't read the second element so that probe keeps working if we
change the DT in the future.
Cc: Ivan T. Ivanov <iivanov.xz@gmail.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
The thermal driver is standalone driver which is used to enable
thermal sensors, so it can be used with any cooling device and
should not bind with CPU cooling device driver.
This original patch is suggested by Amit Kucheria; so it's to
polish the dependency in Kconfig, and remove the dependency with
CPU_THERMAL.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
This patch fixes the following Coccinelle error:
./drivers/thermal/ti-soc-thermal/ti-bandgap.c:1441:1-7: \
ERROR: missing clk_put; clk_get on line 1290 \
and execution via conditional on line 1298
Signed-off-by: Luis Henriques <henrix@camandro.org>
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
As a safety check, this patch changes thermal
core to check for pointer content size, instead of type size,
while allocating memory.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Pass through the code to remove check suggested by
checkpatch.pl (alignment to parenthesis):
CHECK: Alignment should match open parenthesis
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Simplify the GPL notice by removing the FSF address.
No need to track FSF location in this file.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Remove the following warning:
In file included from drivers/thermal/thermal_sysfs.c:19:0:
include/linux/device.h:575:26: warning: ‘dev_attr_emul_temp’ defined but not used [-Wunused-variable]
struct device_attribute dev_attr_##_name = __ATTR(_name, _mode, _show, _store)
^
drivers/thermal/thermal_sysfs.c:395:8: note: in expansion of macro ‘DEVICE_ATTR’
when emul temp is disabled at Kconfig.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Simply marking the power actor section and adding a
comment describing it.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
moving the helper function to closer to similar functions.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Moving the helper to closer where it is used.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Moving the helper to closer where it is used.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Moving the helper to closer where it is used.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Given that idr is only used to get id for thermal devices
(zones and cooling), makes sense to move the code closer.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Organize thermal core code to group the functions
handling with governor manipulation in one single section.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Here we have a simple code organization. This patch moves
functions that do not need to handle thermal core internal
data structure to thermal_helpers.c file.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
This is a code reorganization, simply to concentrate
the sysfs handling functions in thermal_sysfs.c.
This patch moves the cooling device handling functions.
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
