Re: [PATCH] Revert "thermal: mediatek: fix register index error"

[Daniel Lezcano]

On 12/07/2020 18:55, Matthias Brugger wrote:
> On 10/07/2020 15:58, Matthias Brugger wrote:
>>
>>
>> On 07/07/2020 12:34, Enric Balletbo i Serra wrote:
>>> This reverts commit eb9aecd90d1a39601e91cd08b90d5fee51d321a6
>>>
>>> The above patch is supposed to fix a register index error on mt2701. It
>>> is not clear if the problem solved is a hang or just an invalid value
>>> returned, my guess is the second. The patch introduces, though, a new
>>> hang on MT8173 device making them unusable. So, seems reasonable, revert
>>> the patch because introduces a worst issue.
>>>
>>> The reason I send a revert instead of trying to fix the issue for MT8173
>>> is because the information needed to fix the issue is in the datasheet
>>> and is not public. So I am not really able to fix it.
>>>
>>> Fixes the following bug when CONFIG_MTK_THERMAL is set on MT8173
>>> devices.
>>>
>>> [ÂÂÂ 2.222488] Unable to handle kernel paging request at virtual
>>> address ffff8000125f5001
>>> [ÂÂÂ 2.230421] Mem abort info:
>>> [ÂÂÂ 2.233207]ÂÂ ESR = 0x96000021
>>> [ÂÂÂ 2.236261]ÂÂ EC = 0x25: DABT (current EL), IL = 32 bits
>>> [ÂÂÂ 2.241571]ÂÂ SET = 0, FnV = 0
>>> [ÂÂÂ 2.244623]ÂÂ EA = 0, S1PTW = 0
>>> [ÂÂÂ 2.247762] Data abort info:
>>> [ÂÂÂ 2.250640]ÂÂ ISV = 0, ISS = 0x00000021
>>> [ÂÂÂ 2.254473]ÂÂ CM = 0, WnR = 0
>>> [ÂÂÂ 2.257544] swapper pgtable: 4k pages, 48-bit VAs,
>>> pgdp=0000000041850000
>>> [ÂÂÂ 2.264251] [ffff8000125f5001] pgd=000000013ffff003,
>>> pud=000000013fffe003, pmd=000000013fff9003, pte=006800001100b707
>>> [ÂÂÂ 2.274867] Internal error: Oops: 96000021 [#1] PREEMPT SMP
>>> [ÂÂÂ 2.280432] Modules linked in:
>>> [ÂÂÂ 2.283483] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc6+ #162
>>> [ÂÂÂ 2.289914] Hardware name: Google Elm (DT)
>>> [ÂÂÂ 2.294003] pstate: 20000005 (nzCv daif -PAN -UAO)
>>> [ÂÂÂ 2.298792] pc : mtk_read_temp+0xb8/0x1c8
>>> [ÂÂÂ 2.302793] lr : mtk_read_temp+0x7c/0x1c8
>>> [ÂÂÂ 2.306794] sp : ffff80001003b930
>>> [ÂÂÂ 2.310100] x29: ffff80001003b930 x28: 0000000000000000
>>> [ÂÂÂ 2.315404] x27: 0000000000000002 x26: ffff0000f9550b10
>>> [ÂÂÂ 2.320709] x25: ffff0000f9550a80 x24: 0000000000000090
>>> [ÂÂÂ 2.326014] x23: ffff80001003ba24 x22: 00000000610344c0
>>> [ÂÂÂ 2.331318] x21: 0000000000002710 x20: 00000000000001f4
>>> [ÂÂÂ 2.336622] x19: 0000000000030d40 x18: ffff800011742ec0
>>> [ÂÂÂ 2.341926] x17: 0000000000000001 x16: 0000000000000001
>>> [ÂÂÂ 2.347230] x15: ffffffffffffffff x14: ffffff0000000000
>>> [ÂÂÂ 2.352535] x13: ffffffffffffffff x12: 0000000000000028
>>> [ÂÂÂ 2.357839] x11: 0000000000000003 x10: ffff800011295ec8
>>> [ÂÂÂ 2.363143] x9 : 000000000000291b x8 : 0000000000000002
>>> [ÂÂÂ 2.368447] x7 : 00000000000000a8 x6 : 0000000000000004
>>> [ÂÂÂ 2.373751] x5 : 0000000000000000 x4 : ffff800011295cb0
>>> [ÂÂÂ 2.379055] x3 : 0000000000000002 x2 : ffff8000125f5001
>>> [ÂÂÂ 2.384359] x1 : 0000000000000001 x0 : ffff0000f9550a80
>>> [ÂÂÂ 2.389665] Call trace:
>>> [ÂÂÂ 2.392105]Â mtk_read_temp+0xb8/0x1c8
>>> [ÂÂÂ 2.395760]Â of_thermal_get_temp+0x2c/0x40
>>> [ÂÂÂ 2.399849]Â thermal_zone_get_temp+0x78/0x160
>>> [ÂÂÂ 2.404198]Â thermal_zone_device_update.part.0+0x3c/0x1f8
>>> [ÂÂÂ 2.409589]Â thermal_zone_device_update+0x34/0x48
>>> [ÂÂÂ 2.414286]Â of_thermal_set_mode+0x58/0x88
>>> [ÂÂÂ 2.418375]Â thermal_zone_of_sensor_register+0x1a8/0x1d8
>>> [ÂÂÂ 2.423679]Â devm_thermal_zone_of_sensor_register+0x64/0xb0
>>> [ÂÂÂ 2.429242]Â mtk_thermal_probe+0x690/0x7d0
>>> [ÂÂÂ 2.433333]Â platform_drv_probe+0x5c/0xb0
>>> [ÂÂÂ 2.437335]Â really_probe+0xe4/0x448
>>> [ÂÂÂ 2.440901]Â driver_probe_device+0xe8/0x140
>>> [ÂÂÂ 2.445077]Â device_driver_attach+0x7c/0x88
>>> [ÂÂÂ 2.449252]Â __driver_attach+0xac/0x178
>>> [ÂÂÂ 2.453082]Â bus_for_each_dev+0x78/0xc8
>>> [ÂÂÂ 2.456909]Â driver_attach+0x2c/0x38
>>> [ÂÂÂ 2.460476]Â bus_add_driver+0x14c/0x230
>>> [ÂÂÂ 2.464304]Â driver_register+0x6c/0x128
>>> [ÂÂÂ 2.468131]Â __platform_driver_register+0x50/0x60
>>> [ÂÂÂ 2.472831]Â mtk_thermal_driver_init+0x24/0x30
>>> [ÂÂÂ 2.477268]Â do_one_initcall+0x50/0x298
>>> [ÂÂÂ 2.481098]Â kernel_init_freeable+0x1ec/0x264
>>> [ÂÂÂ 2.485450]Â kernel_init+0x1c/0x110
>>> [ÂÂÂ 2.488931]Â ret_from_fork+0x10/0x1c
>>> [ÂÂÂ 2.492502] Code: f9401081 f9400402 b8a67821 8b010042 (b9400042)
>>> [ÂÂÂ 2.498599] ---[ end trace e43e3105ed27dc99 ]---
>>> [ÂÂÂ 2.503367] Kernel panic - not syncing: Attempted to kill init!
>>> exitcode=0x0000000b
>>> [ÂÂÂ 2.511020] SMP: stopping secondary CPUs
>>> [ÂÂÂ 2.514941] Kernel Offset: disabled
>>> [ÂÂÂ 2.518421] CPU features: 0x090002,25006005
>>> [ÂÂÂ 2.522595] Memory Limit: none
>>> [ÂÂÂ 2.525644] ---[ end Kernel panic - not syncing: Attempted to kill
>>> init! exitcode=0x0000000b ]--
>>>
>>> Cc: Michael Kao <michael.kao@xxxxxxxxxxxx>
>>> Fixes: eb9aecd90d1a ("thermal: mediatek: fix register index error")
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx>
>>
>> Reviewed-by: Matthias Brugger <matthias.bgg@xxxxxxxxx>
>>
>> Daniel, Zhang, Amit: can you take this as a bugfix for v5.8 please? We
>> waited long enough to get a proper fix for the driver, but nothing was
>> posted on the mailinglist. Also we don't know if this will break
>> mt2701 or not, we prefer to have a working mt8173 as this is actually
>> a SoC that is available to the general public (as a chromebook product).
>>
>> I propose to take this revert for now and hope that MediaTek will fix
>> the driver for good in the near future.
>>
> 
> Frank tested the patch, with the only board that is affected and
> available (apart from the mt8183 SoC), the BananaPi R2 (mt7623). The
> revert does not break the driver. Even more interesting, with and
> without the revert the thermal sensor returns always zero, so it seems
> it never actually worked.
> 
> So I think we are more then good, to go ahead with reverting the patch.

Ok, I'll take care of it as a fix for 5.8.

Thanks!

  -- Daniel

-- 
<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