[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c7ed6c63-cbb5-07dc-c292-2c473af8c4fb@samsung.com>
Date: Mon, 6 Jul 2020 15:17:08 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>, rui.zhang@...el.com
Cc: srinivas.pandruvada@...ux.intel.com, rkumbako@...eaurora.org,
amit.kucheria@...aro.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org,
Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Subject: Re: [PATCH v4 4/4] thermal: core: Add notifications call in the
framework
Hi Daniel,
On 06.07.2020 12:55, Daniel Lezcano wrote:
> The generic netlink protocol is implemented but the different
> notification functions are not yet connected to the core code.
>
> These changes add the notification calls in the different
> corresponding places.
>
> Reviewed-by: Amit Kucheria <amit.kucheria@...aro.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
This patch landed in today's linux-next 20200706 as commit 5df786e46560
("thermal: core: Add notifications call in the framework"). Sadly it
breaks booting various Samsung Exynos based boards. Here is an example
log from Odroid U3 board:
Unable to handle kernel NULL pointer dereference at virtual address 00000010
pgd = (ptrval)
[00000010] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.8.0-rc3-00015-g5df786e46560
#1146
Hardware name: Samsung Exynos (Flattened Device Tree)
PC is at kmem_cache_alloc+0x13c/0x418
LR is at kmem_cache_alloc+0x48/0x418
pc : [<c02b5cac>] lr : [<c02b5bb8>] psr: 20000053
...
Flags: nzCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment none
Control: 10c5387d Table: 4000404a DAC: 00000051
Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
Stack: (0xee8f1cf8 to 0xee8f2000)
...
[<c02b5cac>] (kmem_cache_alloc) from [<c08cd170>] (__alloc_skb+0x5c/0x170)
[<c08cd170>] (__alloc_skb) from [<c07ec19c>]
(thermal_genl_send_event+0x24/0x174)
[<c07ec19c>] (thermal_genl_send_event) from [<c07ec648>]
(thermal_notify_tz_create+0x58/0x74)
[<c07ec648>] (thermal_notify_tz_create) from [<c07e9058>]
(thermal_zone_device_register+0x358/0x650)
[<c07e9058>] (thermal_zone_device_register) from [<c1028d34>]
(of_parse_thermal_zones+0x304/0x7a4)
[<c1028d34>] (of_parse_thermal_zones) from [<c1028964>]
(thermal_init+0xdc/0x154)
[<c1028964>] (thermal_init) from [<c0102378>] (do_one_initcall+0x8c/0x424)
[<c0102378>] (do_one_initcall) from [<c1001158>]
(kernel_init_freeable+0x190/0x204)
[<c1001158>] (kernel_init_freeable) from [<c0ab85f4>]
(kernel_init+0x8/0x118)
[<c0ab85f4>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
Reverting it on top of linux-next fixes the boot issue. I will
investigate it further soon.
> ---
> v4:
> - Fixed missing static declaration, reported by kbuild-bot
> - Removed max state notification
> ---
> drivers/thermal/thermal_core.c | 21 +++++++++++++++++++++
> drivers/thermal/thermal_helpers.c | 13 +++++++++++--
> drivers/thermal/thermal_sysfs.c | 15 ++++++++++++++-
> 3 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 5fae1621fb01..25ef29123f72 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -215,6 +215,8 @@ int thermal_zone_device_set_policy(struct thermal_zone_device *tz,
> mutex_unlock(&tz->lock);
> mutex_unlock(&thermal_governor_lock);
>
> + thermal_notify_tz_gov_change(tz->id, policy);
> +
> return ret;
> }
>
> @@ -406,12 +408,25 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
> static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
> {
> enum thermal_trip_type type;
> + int trip_temp, hyst = 0;
>
> /* Ignore disabled trip points */
> if (test_bit(trip, &tz->trips_disabled))
> return;
>
> + tz->ops->get_trip_temp(tz, trip, &trip_temp);
> tz->ops->get_trip_type(tz, trip, &type);
> + if (tz->ops->get_trip_hyst)
> + tz->ops->get_trip_hyst(tz, trip, &hyst);
> +
> + if (tz->last_temperature != THERMAL_TEMP_INVALID) {
> + if (tz->last_temperature < trip_temp &&
> + tz->temperature >= trip_temp)
> + thermal_notify_tz_trip_up(tz->id, trip);
> + if (tz->last_temperature >= trip_temp &&
> + tz->temperature < (trip_temp - hyst))
> + thermal_notify_tz_trip_down(tz->id, trip);
> + }
>
> if (type == THERMAL_TRIP_CRITICAL || type == THERMAL_TRIP_HOT)
> handle_critical_trips(tz, trip, type);
> @@ -443,6 +458,8 @@ static void update_temperature(struct thermal_zone_device *tz)
> mutex_unlock(&tz->lock);
>
> trace_thermal_temperature(tz);
> +
> + thermal_genl_sampling_temp(tz->id, temp);
> }
>
> static void thermal_zone_device_init(struct thermal_zone_device *tz)
> @@ -1405,6 +1422,8 @@ thermal_zone_device_register(const char *type, int trips, int mask,
> if (atomic_cmpxchg(&tz->need_update, 1, 0))
> thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>
> + thermal_notify_tz_create(tz->id, tz->type);
> +
> return tz;
>
> unregister:
> @@ -1476,6 +1495,8 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
> ida_destroy(&tz->ida);
> mutex_destroy(&tz->lock);
> device_unregister(&tz->device);
> +
> + thermal_notify_tz_delete(tz->id);
> }
> EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
>
> diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
> index 87b1256fa2f2..c94bc824e5d3 100644
> --- a/drivers/thermal/thermal_helpers.c
> +++ b/drivers/thermal/thermal_helpers.c
> @@ -175,6 +175,16 @@ void thermal_zone_set_trips(struct thermal_zone_device *tz)
> mutex_unlock(&tz->lock);
> }
>
> +static void thermal_cdev_set_cur_state(struct thermal_cooling_device *cdev,
> + int target)
> +{
> + if (cdev->ops->set_cur_state(cdev, target))
> + return;
> +
> + thermal_notify_cdev_state_update(cdev->id, target);
> + thermal_cooling_device_stats_update(cdev, target);
> +}
> +
> void thermal_cdev_update(struct thermal_cooling_device *cdev)
> {
> struct thermal_instance *instance;
> @@ -197,8 +207,7 @@ void thermal_cdev_update(struct thermal_cooling_device *cdev)
> target = instance->target;
> }
>
> - if (!cdev->ops->set_cur_state(cdev, target))
> - thermal_cooling_device_stats_update(cdev, target);
> + thermal_cdev_set_cur_state(cdev, target);
>
> cdev->updated = true;
> mutex_unlock(&cdev->lock);
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index aa99edb4dff7..ff449943f757 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -124,7 +124,8 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
> {
> struct thermal_zone_device *tz = to_thermal_zone(dev);
> int trip, ret;
> - int temperature;
> + int temperature, hyst = 0;
> + enum thermal_trip_type type;
>
> if (!tz->ops->set_trip_temp)
> return -EPERM;
> @@ -139,6 +140,18 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr,
> if (ret)
> return ret;
>
> + if (tz->ops->get_trip_hyst) {
> + ret = tz->ops->get_trip_hyst(tz, trip, &hyst);
> + if (ret)
> + return ret;
> + }
> +
> + ret = tz->ops->get_trip_type(tz, trip, &type);
> + if (ret)
> + return ret;
> +
> + thermal_notify_tz_trip_change(tz->id, trip, type, temperature, hyst);
> +
> thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>
> return count;
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Powered by blists - more mailing lists