On 06/07/2020 15:17, Marek Szyprowski wrote:
> 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.kuche...@linaro.org>
>> Signed-off-by: Daniel Lezcano <daniel.lezc...@linaro.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.
Thanks for reporting this.
Can you send the addr2line result and code it points to ?
>> ---
>> 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
>
--
<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
