Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <4e4d873b-4a40-334a-34e7-defb0fcc4e6b@samsung.com>
Date:   Mon, 3 Oct 2022 14:50:40 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0)
        Gecko/20100101 Thunderbird/91.13.1
From:   Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH v8 19/29] thermal/of: Remove of_thermal_get_crit_temp()
To:     Daniel Lezcano <daniel.lezcano@linaro.org>, rafael@kernel.org
Cc:     linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
        Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Alim Akhtar <alim.akhtar@samsung.com>,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, linux-tegra@vger.kernel.org,
        linux-omap@vger.kernel.org
Content-Language: en-US
In-Reply-To: <20221003092602.1323944-20-daniel.lezcano@linaro.org>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20221003092602.1323944-1-daniel.lezcano@linaro.org>
        <CGME20221003093207eucas1p1d456288f35eadbc6fcda0bf24b58e678@eucas1p1.samsung.com>
        <20221003092602.1323944-20-daniel.lezcano@linaro.org>
Precedence: bulk

Hi Daniel,

On 03.10.2022 11:25, Daniel Lezcano wrote:
> The generic version of of_thermal_get_crit_temp() can be used. Let's
> remove this ops which is pointless.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

This patch breaks Exynos thermal driver as it introduces a NULL pointer 
dereference in exynos_tmu_initialize():

8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address 00000000
[00000000] *pgd=00000000
Internal error: Oops: 80000005 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.0.0-rc1-00072-ge521efddb107 
#12941
Hardware name: Samsung Exynos (Flattened Device Tree)
dwc2 12480000.hsotg: new address 125
PC is at 0x0
LR is at exynos_tmu_initialize+0x4c/0x1e8
...
Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
Stack: (0xf082dd78 to 0xf082e000)
...
  exynos_tmu_initialize from exynos_tmu_probe+0x2b0/0x728
  exynos_tmu_probe from platform_probe+0x5c/0xb8
  platform_probe from really_probe+0xe0/0x414
  really_probe from __driver_probe_device+0xa0/0x208
  __driver_probe_device from driver_probe_device+0x30/0xc0
  driver_probe_device from __driver_attach+0xf0/0x1f0
  __driver_attach from bus_for_each_dev+0x70/0xb0
  bus_for_each_dev from bus_add_driver+0x174/0x218
  bus_add_driver from driver_register+0x88/0x11c
  driver_register from do_one_initcall+0x64/0x380
  do_one_initcall from kernel_init_freeable+0x1c0/0x224
  kernel_init_freeable from kernel_init+0x18/0x12c
  kernel_init from ret_from_fork+0x14/0x2c
Exception stack(0xf082dfb0 to 0xf082dff8)
...
Code: bad PC value
---[ end trace 0000000000000000 ]---

If there is no replacement for tzd->ops->get_crit_temp(tzd, &temp), then 
please simply remove that call in exynos_tmu_initialize() to avoid 
breaking the initialization.

> ---
> drivers/thermal/thermal_of.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 494e9c319541..bd872183e521 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -19,20 +19,6 @@
> #include "thermal_core.h"
> -static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
> - int *temp)
> -{
> - int i;
> -
> - for (i = 0; i < tz->num_trips; i++)
> - if (tz->trips[i].type == THERMAL_TRIP_CRITICAL) {
> - *temp = tz->trips[i].temperature;
> - return 0;
> - }
> -
> - return -EINVAL;
> -}
> -
> /*** functions parsing device tree nodes ***/
> static int of_find_trip_id(struct device_node *np, struct device_node 
> *trip)
> @@ -529,7 +515,6 @@ struct thermal_zone_device 
> *thermal_of_zone_register(struct device_node *sensor,
> goto out_kfree_trips;
> }
> - of_ops->get_crit_temp = of_ops->get_crit_temp ? : 
> of_thermal_get_crit_temp;
> of_ops->bind = thermal_of_bind;
> of_ops->unbind = thermal_of_unbind;

Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland