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

Dmitry Baryshkov
2020-06-04T15:03:55Z

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