Re: [PATCH 5/7] arm64: dts: qcom: pm8150x: add thermal alarms and thermal zones

[Dmitry Baryshkov <dmitry.baryshkov@linaro.org>]

On 04/06/2020 13:47, Vinod Koul wrote:
> On 04-06-20, 03:43, Dmitry Baryshkov wrote:
>> Add temperature alarm and thermal zone configuration to all three
>> pm8150 instances. Configuration is largely based on the msm-4.19 tree.
>> These alarms use main adc of the pmic. Separate temperature adc is not
>> supported yet.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/pm8150.dtsi  | 41 +++++++++++++++++++++++--
>>   arch/arm64/boot/dts/qcom/pm8150b.dtsi | 43 +++++++++++++++++++++++++--
>>   arch/arm64/boot/dts/qcom/pm8150l.dtsi | 43 +++++++++++++++++++++++++--
>>   3 files changed, 119 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/pm8150.dtsi b/arch/arm64/boot/dts/qcom/pm8150.dtsi
>> index c0b197458665..fee2db42f4cb 100644
>> --- a/arch/arm64/boot/dts/qcom/pm8150.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/pm8150.dtsi
>> @@ -30,6 +30,15 @@ pwrkey {
>>   			};
>>   		};
>>   
>> +		pm8150_temp: temp-alarm@2400 {
>> +			compatible = "qcom,spmi-temp-alarm";
>> +			reg = <0x2400>;
>> +			interrupts = <0x0 0x24 0x0 IRQ_TYPE_EDGE_BOTH>;
>> +			io-channels = <&pm8150_adc ADC5_DIE_TEMP>;
>> +			io-channel-names = "thermal";
>> +			#thermal-sensor-cells = <0>;
>> +		};
>> +
>>   		pm8150_adc: adc@3100 {
>>   			compatible = "qcom,spmi-adc5";
>>   			reg = <0x3100>;
>> @@ -38,8 +47,6 @@ pm8150_adc: adc@3100 {
>>   			#io-channel-cells = <1>;
>>   			interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
>>   
>> -			status = "disabled";
>> -
> 
> This should not be removed, rather than this please add enabled in you
> board dts file
> 
>>   			ref-gnd@0 {
>>   				reg = <ADC5_REF_GND>;
>>   				qcom,pre-scaling = <1 1>;
>> @@ -85,3 +92,33 @@ pmic@1 {
>>   		#size-cells = <0>;
>>   	};
>>   };
>> +
>> +&thermal_zones {
>> +	pm8150_temp {
>> +		polling-delay-passive = <0>;
>> +		polling-delay = <0>;
>> +
>> +		thermal-sensors = <&pm8150_temp>;
>> +
>> +		trips {
>> +			trip0 {
>> +				temperature = <95000>;
>> +				hysteresis = <0>;
>> +				type = "passive";
>> +			};
>> +
>> +			trip1 {
>> +				temperature = <115000>;
>> +				hysteresis = <0>;
>> +				type = "passive";
>> +			};
>> +
>> +			trip2 {
>> +				temperature = <145000>;
>> +				hysteresis = <0>;
>> +				type = "passive";
>> +			};
>> +		};
>> +
>> +	};
> 
> Not sure about this, Amit..? Should this also not be in board dts?
> 
> Similar comments on similar ones for rest of the patch as well..

I'm not so sure. This part of the configuration seems generic to me. 
Unlike adc-tm config, which definitely goes to the board file.

I can split this into a separate pm8150-temp.dtsi file. Does that sound 
better?


-- 
With best wishes
Dmitry