lkml.org 
[lkml]   [2023]   [Oct]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 4/4] arm64: dts: qcom: qcm6490-fairphone-fp5: Add PM7325 thermals
From


On 10/20/23 13:31, Luca Weiss wrote:
> On Wed Oct 18, 2023 at 10:28 PM CEST, Konrad Dybcio wrote:
>>
>>
>> On 10/14/23 19:52, Luca Weiss wrote:
>>> On Samstag, 14. Oktober 2023 01:13:29 CEST Konrad Dybcio wrote:
>>>> On 13.10.2023 10:09, Luca Weiss wrote:
>>>>> Configure the thermals for the QUIET_THERM, CAM_FLASH_THERM, MSM_THERM
>>>>> and RFC_CAM_THERM thermistors connected to PM7325.
>>>>>
>>>>> With this PMIC the software communication to the ADC is going through
>>>>> PMK7325 (= PMK8350).
>>>>>
>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>>> ---
>>>>>
>>>>> arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 117
>>>>> +++++++++++++++++++++ 1 file changed, 117 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>>>>> b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts index
>>>>> 2c01f799a6b2..d0b1e4e507ff 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>>>>> +++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
>>>>> @@ -9,6 +9,7 @@
>>>>>
>>>>> #define PM7250B_SID 8
>>>>> #define PM7250B_SID1 9
>>>>>
>>>>> +#include <dt-bindings/iio/qcom,spmi-adc7-pm7325.h>
>>>>>
>>>>> #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
>>>>> #include <dt-bindings/leds/common.h>
>>>>> #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>>>>>
>>>>> @@ -137,6 +138,20 @@ afvdd_2p8: regulator-afvdd-2p8 {
>>>>>
>>>>> };
>>>>>
>>>>> thermal-zones {
>>>>>
>>>>> + camera-thermal {
>>>>> + polling-delay-passive = <0>;
>>>>> + polling-delay = <0>;
>>>>> + thermal-sensors = <&pmk8350_adc_tm 2>;
>>>>> +
>>>>> + trips {
>>>>> + active-config0 {
>>>>> + temperature = <125000>;
>>>>
>>>> are
>>>>
>>>>> + rear-cam-thermal {
>>>>>
>>>>> + temperature = <125000>;
>>>>
>>>> you
>>>>
>>>>> + sdm-skin-thermal {
>>>>>
>>>>> + temperature = <125000>;
>>>>
>>>> sure
>>>>
>>>> about these temps?
>>>
>>> (email from my other address, quicker right now)
>>>
>>> Well yes and no.
>>>
>>> Yes as in those are the temps specified in downstream dtb.
>>> No as in I'm 99% sure there's user space with definitely lower threshold that
>>> actually does something in response to the temps.
>>>
>>> I didn't look too much into this but does the kernel even do something when it
>>> hits one of these trip points? I assume when there's a cooling device thing
>>> specified then it can actually tell the driver to do something, but without
>>> (and most drivers don't support this?) I'm assuming the kernel can't do much
>>> anyways?
>>>
>>> So e.g. when the temperature for the flash led is reached I'm assuming
>>> downstream (+Android) either dims the led or turns it off? But I'd have to dig
>>> quite a bit into the thermal setup there to check what it's really doing.
>> I think reaching "critical" shuts down the platform, unless something
>> registering the thermal zone explicitly overrides the behavior.
>
> Should probably be easy to test, especially the camera flash thermal
> zone heats up *very* quickly when the flash is on, so should be trivial
> to set the trip point down from 125degC to e.g. 45degC and see what
> happens.
>
> So I did this and... nothing happened.
> I watched /sys/class/thermal/thermal_zone34/temp climb above 45degC and
> nothing happened.
>
> I guess trip type being "passive" and no cooling-device makes it not do
> anything.
>
> ==> /sys/class/thermal/thermal_zone34/trip_point_0_hyst <==
> 1000
> ==> /sys/class/thermal/thermal_zone34/trip_point_0_temp <==
> 45000
> ==> /sys/class/thermal/thermal_zone34/trip_point_0_type <==
> passive
>
> From Documentation/devicetree/bindings/thermal/thermal-zones.yaml:
>
> - active # enable active cooling e.g. fans
> - passive # enable passive cooling e.g. throttling cpu
> - hot # send notification to driver
> - critical # send notification to driver, trigger shutdown
>
> So unless we want to just shut down the system (with "critical"), I
> don't think thermal can't really do anything else right now, since e.g.
> leds-qcom-flash.c driver doesn't have any cooling support to lower the
> brightness or turn off the LED.
>
> So.. in essence not much we can do right now.
Yeah.. crashing the phone because the LED is too hot is sorta
suboptimal! Though I mainly had the skin temp in mind..

>
> But seems we also cannot remove this (kinda useless) trip since we need
> at least one trip point in the dts if I read the bindings yaml
> correctly.
Right

>
>>
>>>
>>> But for now I think it's okay to put this current thermal config into dts and
>>> we'll improve it later when 1. I understand more and 2. maybe some useful
>>> drivers support the cooling bits?
>> Yeah it's better than nothing, but ultimately we should probably move
>> the values that userspace daemon operates on here in the dt..
>
> For sure.. I spent a bit of time looking into the proprietary Qualcomm
> thermal-daemon sources but didn't really see much interesting things
> there for this platform, maybe some of this thermal handling is
> somewhere else - or half of these thermal zones aren't even used with
> Android.
>
> So.. good to get the current patch upstream or not? :)
Yep, just having the ability to read out thing is always good ;)

Konrad

\
 
 \ /
  Last update: 2023-10-21 18:23    [W:0.076 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site