On 10/19/21 12:35 AM, Daniel Lezcano wrote: > On 19/10/2021 03:21, Subbaraman Narayanamurthy wrote: >> On 10/8/21 12:50 PM, Subbaraman Narayanamurthy wrote: >>> On 10/6/21 4:08 AM, Daniel Lezcano wrote: > [ ... ] > >>> /sys/devices/virtual/thermal/thermal_zone87 # echo 120000 > trip_point_0_temp >>> [ 184.290964][ T211] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000020 >>> [ 184.300896][ T211] Mem abort info: >>> [ 184.304486][ T211] ESR = 0x96000006 >>> [ 184.308348][ T211] EC = 0x25: DABT (current EL), IL = 32 bits >>> [ 184.314531][ T211] SET = 0, FnV = 0 >>> [ 184.318384][ T211] EA = 0, S1PTW = 0 >>> [ 184.322323][ T211] Data abort info: >>> [ 184.325993][ T211] ISV = 0, ISS = 0x00000006 >>> [ 184.330655][ T211] CM = 0, WnR = 0 >>> [ 184.334425][ T211] user pgtable: 4k pages, 39-bit VAs, pgdp=000000081a7a2000 >>> [ 184.341750][ T211] [0000000000000020] pgd=000000081a7a7003, p4d=000000081a7a7003, pud=000000081a7a7003, pmd=0000000000000000 >>> [ 184.353359][ T211] Internal error: Oops: 96000006 [#1] PREEMPT SMP >>> [ 184.359797][ T211] Dumping ftrace buffer: >>> [ 184.364001][ T211] (ftrace buffer empty) >>> >>> Hope this helps. >> Hi Daniel, >> Have you got a chance to look at this? > Hi Subbaraman, > > Actually, I think the root problem is the thermal zone is showing up > while there is no sensor associated with it. You can read the > temperature and get a kernel warning also. > > That's what should be fixed IMO. > Hi Daniel, If I understand your statement correctly, are you saying that a thermal_zone device should be created only after a thermal sensor driver supplying it probes? >From what I can see, thermal_init() --> of_parse_thermal_zones() --> thermal_zone_device_register() --> thermal_zone_create_device_groups() <followed by> device_register() with thermal_zone%d This happens way before a thermal sensor driver probes. So, creating a thermal_zone device only after its thermal sensor probes would require more changes to the framework. Also, I see a similar NULL check exists in the framework code already. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/thermal/thermal_of.c?h=v5.15-rc6#n103 So, extending the logic for other callsites (below) makes sense to me. - of_thermal_get_temp() - of_thermal_set_emul_temp() - of_thermal_get_trend() - of_thermal_set_trip_temp() Thanks, Subbaraman