[PATCH 1/7] arm64: dts: qcom: pm8009: Add base dts file

[PATCH 1/7] arm64: dts: qcom: pm8009: Add base dts file

[Dmitry Baryshkov]

Add base DTS file for pm8009 along with GPIOs and power-on nodes.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pm8009.dtsi | 40 ++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/pm8009.dtsi

diff --git a/arch/arm64/boot/dts/qcom/pm8009.dtsi b/arch/arm64/boot/dts/qcom/pm8009.dtsi
new file mode 100644
index 000000000000..9f3e19b5bd00
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/pm8009.dtsi
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2019, Linaro Limited
+ */
+
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/spmi/spmi.h>
+#include <dt-bindings/iio/qcom,spmi-vadc.h>
+
+&spmi_bus {
+	pmic@a {
+		compatible = "qcom,pm8009", "qcom,spmi-pmic";
+		reg = <0xa SPMI_USID>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		power-on@800 {
+			compatible = "qcom,pm8916-pon";
+			reg = <0x0800>;
+		};
+
+		pm8009_gpios: gpio@c000 {
+			compatible = "qcom,pm8005-gpio";
+			reg = <0xc000>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+	};
+
+	pmic@b {
+		compatible = "qcom,pm8009", "qcom,spmi-pmic";
+		reg = <0xb SPMI_USID>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+};
-- 
2.26.2

[PATCH 2/7] arm64: dts: qcom: sm8250: change spmi node label

[Dmitry Baryshkov]

PMIC dtsi files (pm8150*.dtsi) expect to have spmi_bus label, rather
than just spmi. Rename spmi label accordingly.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 1e2862bbfb11..9dd27aecdfda 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -991,7 +991,7 @@ pdc: interrupt-controller@b220000 {
 			interrupt-controller;
 		};
 
-		spmi: qcom,spmi@c440000 {
+		spmi_bus: qcom,spmi@c440000 {
 			compatible = "qcom,spmi-pmic-arb";
 			reg = <0x0 0x0c440000 0x0 0x0001100>,
 			      <0x0 0x0c600000 0x0 0x2000000>,
-- 
2.26.2

Re: [PATCH 2/7] arm64: dts: qcom: sm8250: change spmi node label

[Manivannan Sadhasivam]

On Thu, Jun 04, 2020 at 03:43:26AM +0300, Dmitry Baryshkov wrote:
> PMIC dtsi files (pm8150*.dtsi) expect to have spmi_bus label, rather
> than just spmi. Rename spmi label accordingly.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 1e2862bbfb11..9dd27aecdfda 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -991,7 +991,7 @@ pdc: interrupt-controller@b220000 {
>  			interrupt-controller;
>  		};
>  
> -		spmi: qcom,spmi@c440000 {
> +		spmi_bus: qcom,spmi@c440000 {
>  			compatible = "qcom,spmi-pmic-arb";
>  			reg = <0x0 0x0c440000 0x0 0x0001100>,
>  			      <0x0 0x0c600000 0x0 0x2000000>,
> -- 
> 2.26.2
> 

[PATCH 3/7] arm64: dts: qcom: sm8250-mtp: include pmic files

[Dmitry Baryshkov]

Include files describing used PMIC chips, GPIOs, power on, etc nodes.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250-mtp.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
index 0bc4b1e037c4..c20ae91d3026 100644
--- a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts
@@ -7,6 +7,10 @@
 
 #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
 #include "sm8250.dtsi"
+#include "pm8150.dtsi"
+#include "pm8150b.dtsi"
+#include "pm8150l.dtsi"
+#include "pm8009.dtsi"
 
 / {
 	model = "Qualcomm Technologies, Inc. SM8250 MTP";
-- 
2.26.2

[PATCH 4/7] arm64: dts: qcom: sm8x50: add empty thermal zone nodes

[Dmitry Baryshkov]

Add empty thermal_zone nodes. They will be populated by PMIC dtsi files,
describing thermal sensors and trip points for zones handled by PMICs.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8150.dtsi | 3 +++
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index 141c21dfa68c..869a54352d77 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -857,6 +857,9 @@ cpufreq_hw: cpufreq@18323000 {
 		};
 	};
 
+	thermal_zones: thermal-zones {
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 1 IRQ_TYPE_LEVEL_LOW>,
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 9dd27aecdfda..972d8e04c8a2 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -1736,6 +1736,9 @@ cpufreq_hw: cpufreq@18591000 {
 		};
 	};
 
+	thermal_zones: thermal-zones {
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 13
-- 
2.26.2

Re: [PATCH 4/7] arm64: dts: qcom: sm8x50: add empty thermal zone nodes

[Amit Kucheria]

On Thu, Jun 4, 2020 at 6:13 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Add empty thermal_zone nodes. They will be populated by PMIC dtsi files,
> describing thermal sensors and trip points for zones handled by PMICs.

This is unnecessary. I've already started preparing patches that add
the thermal zones for the two socs.

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8150.dtsi | 3 +++
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> index 141c21dfa68c..869a54352d77 100644
> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> @@ -857,6 +857,9 @@ cpufreq_hw: cpufreq@18323000 {
>                 };
>         };
>
> +       thermal_zones: thermal-zones {
> +       };
> +
>         timer {
>                 compatible = "arm,armv8-timer";
>                 interrupts = <GIC_PPI 1 IRQ_TYPE_LEVEL_LOW>,
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 9dd27aecdfda..972d8e04c8a2 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -1736,6 +1736,9 @@ cpufreq_hw: cpufreq@18591000 {
>                 };
>         };
>
> +       thermal_zones: thermal-zones {
> +       };
> +
>         timer {
>                 compatible = "arm,armv8-timer";
>                 interrupts = <GIC_PPI 13
> --
> 2.26.2
>

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

[Dmitry Baryshkov]

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";
-
 			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";
+			};
+		};
+
+	};
+};
diff --git a/arch/arm64/boot/dts/qcom/pm8150b.dtsi b/arch/arm64/boot/dts/qcom/pm8150b.dtsi
index 40b5d75a4a1d..e93d16f2d1e0 100644
--- a/arch/arm64/boot/dts/qcom/pm8150b.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8150b.dtsi
@@ -22,7 +22,16 @@ power-on@800 {
 			status = "disabled";
 		};
 
-		adc@3100 {
+		pm8150b_temp: temp-alarm@2400 {
+			compatible = "qcom,spmi-temp-alarm";
+			reg = <0x2400>;
+			interrupts = <0x2 0x24 0x0 IRQ_TYPE_EDGE_BOTH>;
+			io-channels = <&pm8150b_adc ADC5_DIE_TEMP>;
+			io-channel-names = "thermal";
+			#thermal-sensor-cells = <0>;
+		};
+
+		pm8150b_adc: adc@3100 {
 			compatible = "qcom,spmi-adc5";
 			reg = <0x3100>;
 			#address-cells = <1>;
@@ -30,8 +39,6 @@ adc@3100 {
 			#io-channel-cells = <1>;
 			interrupts = <0x2 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
 
-			status = "disabled";
-
 			ref-gnd@0 {
 				reg = <ADC5_REF_GND>;
 				qcom,pre-scaling = <1 1>;
@@ -74,3 +81,33 @@ pmic@3 {
 		#size-cells = <0>;
 	};
 };
+
+&thermal_zones {
+	pm8150b_temp {
+		polling-delay-passive = <0>;
+		polling-delay = <0>;
+
+		thermal-sensors = <&pm8150b_temp>;
+
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+
+			trip2 {
+				temperature = <145000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+		};
+
+	};
+};
diff --git a/arch/arm64/boot/dts/qcom/pm8150l.dtsi b/arch/arm64/boot/dts/qcom/pm8150l.dtsi
index cf05e0685d10..1edf87c95a27 100644
--- a/arch/arm64/boot/dts/qcom/pm8150l.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8150l.dtsi
@@ -22,7 +22,16 @@ power-on@800 {
 			status = "disabled";
 		};
 
-		adc@3100 {
+		pm8150l_temp: temp-alarm@2400 {
+			compatible = "qcom,spmi-temp-alarm";
+			reg = <0x2400>;
+			interrupts = <0x4 0x24 0x0 IRQ_TYPE_EDGE_BOTH>;
+			io-channels = <&pm8150l_adc ADC5_DIE_TEMP>;
+			io-channel-names = "thermal";
+			#thermal-sensor-cells = <0>;
+		};
+
+		pm8150l_adc: adc@3100 {
 			compatible = "qcom,spmi-adc5";
 			reg = <0x3100>;
 			#address-cells = <1>;
@@ -30,8 +39,6 @@ adc@3100 {
 			#io-channel-cells = <1>;
 			interrupts = <0x4 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
 
-			status = "disabled";
-
 			ref-gnd@0 {
 				reg = <ADC5_REF_GND>;
 				qcom,pre-scaling = <1 1>;
@@ -68,3 +75,33 @@ pmic@5 {
 		#size-cells = <0>;
 	};
 };
+
+&thermal_zones {
+	pm8150l_temp {
+		polling-delay-passive = <0>;
+		polling-delay = <0>;
+
+		thermal-sensors = <&pm8150l_temp>;
+
+		trips {
+			trip0 {
+				temperature = <95000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+
+			trip1 {
+				temperature = <115000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+
+			trip2 {
+				temperature = <145000>;
+				hysteresis = <0>;
+				type = "passive";
+			};
+		};
+
+	};
+};
-- 
2.26.2

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

[Vinod Koul]

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

-- 
~Vinod

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

[Dmitry Baryshkov]

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

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

[Vinod Koul]

Sorry missed ccing Amit, done now.

On 04-06-20, 18:03, Dmitry Baryshkov wrote:
> On 04/06/2020 13:47, Vinod Koul wrote:
> > On 04-06-20, 03:43, Dmitry Baryshkov wrote:

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

...

> > > +&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 think the temperature values may be board specific, Amit can confirm
that. If that is the case then this belongs to board dts, otherwise here :)

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

That might make it worse, we don't do splitting.

-- 
~Vinod

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

[Dmitry Baryshkov]

Hello,

On Fri, 5 Jun 2020 at 07:40, Vinod Koul <vkoul@kernel.org> wrote:
>
>
> Sorry missed ccing Amit, done now.
>
> On 04-06-20, 18:03, Dmitry Baryshkov wrote:
> > On 04/06/2020 13:47, Vinod Koul wrote:
> > > On 04-06-20, 03:43, Dmitry Baryshkov wrote:
>
> > > >                   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

Compare this with pm8998.dtsi. It has all nodes enabled by default.

> > > > +&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 think the temperature values may be board specific, Amit can confirm
> that. If that is the case then this belongs to board dts, otherwise here :)

Again, pm8998 has these thermal values in the dtsi file. In V2 I will
update these three files to follow pm8998.dtsi.

-- 
With best wishes
Dmitry

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

[Amit Kucheria]

On Fri, Jun 5, 2020 at 10:10 AM Vinod Koul <vkoul@kernel.org> wrote:
>
>
> Sorry missed ccing Amit, done now.
>
> On 04-06-20, 18:03, Dmitry Baryshkov wrote:
> > On 04/06/2020 13:47, Vinod Koul wrote:
> > > On 04-06-20, 03:43, Dmitry Baryshkov wrote:
>
> > > >                   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

Is the default disabled for a reason?

I'd expect the default to be enabled and then board-specific dts to
disable a specific adc if needed.

> ...
>
> > > > +&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";

Since there is not cooling map associated with these two trips i.e. no
mitigation action, this trip is informational. So make it of type
"hot".

Is there really a need for two passive trip points? Just one at 115
should be enough?

> > > > +                 };
> > > > +
> > > > +                 trip2 {
> > > > +                         temperature = <145000>;

Are you sure about this? That is a very toasty temperature. :-)

> > > > +                         hysteresis = <0>;
> > > > +                         type = "passive";

The last trip should typically be of type "critical". That is the
temperature at which the system will initiate a shutdown.

> > > > +                 };
> > > > +         };
> > > > +
> > > > + };
> > >
> > > 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 think the temperature values may be board specific, Amit can confirm
> that. If that is the case then this belongs to board dts, otherwise here :)

While the temp values can be board-specific e.g. if the same SoC is
used in a mobile phone and a laptop, the thresholds rarely change, in
my experience.

I think they can stay in the pmic dtsi file and any specific board can
override if necessary.

> > I can split this into a separate pm8150-temp.dtsi file. Does that sound
> > better?
>
> That might make it worse, we don't do splitting.

Right, let's not split it.

>
> --
> ~Vinod

[PATCH 6/7] arm64: dts: qcom: pm8150: enable rtc device

[Dmitry Baryshkov]

Enable rtc device provided by the pmic.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/pm8150.dtsi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8150.dtsi b/arch/arm64/boot/dts/qcom/pm8150.dtsi
index fee2db42f4cb..762fb902db81 100644
--- a/arch/arm64/boot/dts/qcom/pm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8150.dtsi
@@ -71,8 +71,6 @@ rtc@6000 {
 			reg = <0x6000>;
 			reg-names = "rtc", "alarm";
 			interrupts = <0x0 0x61 0x1 IRQ_TYPE_NONE>;
-
-			status = "disabled";
 		};
 
 		pm8150_gpios: gpio@c000 {
-- 
2.26.2

Re: [PATCH 6/7] arm64: dts: qcom: pm8150: enable rtc device

[Manivannan Sadhasivam]

On Thu, Jun 04, 2020 at 03:43:30AM +0300, Dmitry Baryshkov wrote:
> Enable rtc device provided by the pmic.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  arch/arm64/boot/dts/qcom/pm8150.dtsi | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm8150.dtsi b/arch/arm64/boot/dts/qcom/pm8150.dtsi
> index fee2db42f4cb..762fb902db81 100644
> --- a/arch/arm64/boot/dts/qcom/pm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm8150.dtsi
> @@ -71,8 +71,6 @@ rtc@6000 {
>  			reg = <0x6000>;
>  			reg-names = "rtc", "alarm";
>  			interrupts = <0x0 0x61 0x1 IRQ_TYPE_NONE>;
> -
> -			status = "disabled";
>  		};
>  
>  		pm8150_gpios: gpio@c000 {
> -- 
> 2.26.2
> 

Re: [PATCH 6/7] arm64: dts: qcom: pm8150: enable rtc device

[Vinod Koul]

On 04-06-20, 03:43, Dmitry Baryshkov wrote:
> Enable rtc device provided by the pmic.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/pm8150.dtsi | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm8150.dtsi b/arch/arm64/boot/dts/qcom/pm8150.dtsi
> index fee2db42f4cb..762fb902db81 100644
> --- a/arch/arm64/boot/dts/qcom/pm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm8150.dtsi
> @@ -71,8 +71,6 @@ rtc@6000 {
>  			reg = <0x6000>;
>  			reg-names = "rtc", "alarm";
>  			interrupts = <0x0 0x61 0x1 IRQ_TYPE_NONE>;
> -
> -			status = "disabled";

Again, pls enable in your board dts...

Thanks

-- 
~Vinod

[PATCH 7/7] arm64: dts: qcom: sm8250: add watchdog device

[Dmitry Baryshkov]

Add on-SoC watchdog device node.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 972d8e04c8a2..f1641c6fe203 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -1662,6 +1662,12 @@ config {
 			};
 		};
 
+		watchdog@17c10000 {
+			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
+			reg = <0 0x17c10000 0 0x1000>;
+			clocks = <&sleep_clk>;
+		};
+
 		timer@17c20000 {
 			#address-cells = <2>;
 			#size-cells = <2>;
-- 
2.26.2

Re: [PATCH 7/7] arm64: dts: qcom: sm8250: add watchdog device

[Manivannan Sadhasivam]

On Thu, Jun 04, 2020 at 03:43:31AM +0300, Dmitry Baryshkov wrote:
> Add on-SoC watchdog device node.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 972d8e04c8a2..f1641c6fe203 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -1662,6 +1662,12 @@ config {
>  			};
>  		};
>  
> +		watchdog@17c10000 {
> +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
> +			reg = <0 0x17c10000 0 0x1000>;
> +			clocks = <&sleep_clk>;
> +		};
> +
>  		timer@17c20000 {
>  			#address-cells = <2>;
>  			#size-cells = <2>;
> -- 
> 2.26.2
> 

Re: [PATCH 7/7] arm64: dts: qcom: sm8250: add watchdog device

[Vinod Koul]

On 04-06-20, 03:43, Dmitry Baryshkov wrote:
> Add on-SoC watchdog device node.

Reviewed-by: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod

Re: [PATCH 7/7] arm64: dts: qcom: sm8250: add watchdog device

[Sai Prakash Ranjan]

On 2020-06-04 06:13, Dmitry Baryshkov wrote:
> Add on-SoC watchdog device node.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 972d8e04c8a2..f1641c6fe203 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -1662,6 +1662,12 @@ config {
>  			};
>  		};
> 
> +		watchdog@17c10000 {
> +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";

Need to add this compatible to bindings.

Thanks,
Sai
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 7/7] arm64: dts: qcom: sm8250: add watchdog device

[Manivannan Sadhasivam]

On Thu, Jun 04, 2020 at 04:37:01PM +0530, Sai Prakash Ranjan wrote:
> On 2020-06-04 06:13, Dmitry Baryshkov wrote:
> > Add on-SoC watchdog device node.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > index 972d8e04c8a2..f1641c6fe203 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > @@ -1662,6 +1662,12 @@ config {
> >  			};
> >  		};
> > 
> > +		watchdog@17c10000 {
> > +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
> 
> Need to add this compatible to bindings.
> 

I did look into this but the binding says,
"compatible : shall contain only one of the following"

So clearly the fallback is not going to work and there is no need to add a
dedicated compatible in the driver. The binding is not in the YAML format to
be validated but still... Other option is to convert the binding to YAML and
make the compatibles listed as 'enum' and 'const' elements appropriately.

Thanks,
Mani

> Thanks,
> Sai
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 7/7] arm64: dts: qcom: sm8250: add watchdog device

[Sai Prakash Ranjan]

On 2020-06-04 17:05, Manivannan Sadhasivam wrote:
> On Thu, Jun 04, 2020 at 04:37:01PM +0530, Sai Prakash Ranjan wrote:
>> On 2020-06-04 06:13, Dmitry Baryshkov wrote:
>> > Add on-SoC watchdog device node.
>> >
>> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> > ---
>> >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > index 972d8e04c8a2..f1641c6fe203 100644
>> > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > @@ -1662,6 +1662,12 @@ config {
>> >  			};
>> >  		};
>> >
>> > +		watchdog@17c10000 {
>> > +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
>> 
>> Need to add this compatible to bindings.
>> 
> 
> I did look into this but the binding says,
> "compatible : shall contain only one of the following"
> 
> So clearly the fallback is not going to work and there is no need to 
> add a
> dedicated compatible in the driver. The binding is not in the YAML 
> format to
> be validated but still... Other option is to convert the binding to 
> YAML and
> make the compatibles listed as 'enum' and 'const' elements 
> appropriately.
> 

I already converted the bindings to YAML and added the missing 
compatibles for
other SoCs[1] . I thought it was already merged since Rob has already 
reviewed
them, but seems like it was missed. Bjorn, can you please take it?

[1] https://lore.kernel.org/patchwork/cover/1192757/

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 7/7] arm64: dts: qcom: sm8250: add watchdog device

[Manivannan Sadhasivam]

On Thu, Jun 04, 2020 at 05:21:46PM +0530, Sai Prakash Ranjan wrote:
> On 2020-06-04 17:05, Manivannan Sadhasivam wrote:
> > On Thu, Jun 04, 2020 at 04:37:01PM +0530, Sai Prakash Ranjan wrote:
> > > On 2020-06-04 06:13, Dmitry Baryshkov wrote:
> > > > Add on-SoC watchdog device node.
> > > >
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > index 972d8e04c8a2..f1641c6fe203 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > > > @@ -1662,6 +1662,12 @@ config {
> > > >  			};
> > > >  		};
> > > >
> > > > +		watchdog@17c10000 {
> > > > +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
> > > 
> > > Need to add this compatible to bindings.
> > > 
> > 
> > I did look into this but the binding says,
> > "compatible : shall contain only one of the following"
> > 
> > So clearly the fallback is not going to work and there is no need to add
> > a
> > dedicated compatible in the driver. The binding is not in the YAML
> > format to
> > be validated but still... Other option is to convert the binding to YAML
> > and
> > make the compatibles listed as 'enum' and 'const' elements
> > appropriately.
> > 
> 
> I already converted the bindings to YAML and added the missing compatibles
> for
> other SoCs[1] .

Ah, okay. I didn't find it in linux-next. Anyway, for adding
"qcom,apss-wdt-sm8250" compatible, we need to group the compatibles wisely
using 'enum' and 'const'.

> I thought it was already merged since Rob has already
> reviewed
> them, but seems like it was missed. Bjorn, can you please take it?
> 

Perhaps for v5.9...

Thanks,
Mani

> [1] https://lore.kernel.org/patchwork/cover/1192757/
> 
> Thanks,
> Sai
> 
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 7/7] arm64: dts: qcom: sm8250: add watchdog device

[Sai Prakash Ranjan]

On 2020-06-04 17:52, Manivannan Sadhasivam wrote:
> On Thu, Jun 04, 2020 at 05:21:46PM +0530, Sai Prakash Ranjan wrote:
>> On 2020-06-04 17:05, Manivannan Sadhasivam wrote:
>> > On Thu, Jun 04, 2020 at 04:37:01PM +0530, Sai Prakash Ranjan wrote:
>> > > On 2020-06-04 06:13, Dmitry Baryshkov wrote:
>> > > > Add on-SoC watchdog device node.
>> > > >
>> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> > > > ---
>> > > >  arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++++++
>> > > >  1 file changed, 6 insertions(+)
>> > > >
>> > > > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > > > b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > > > index 972d8e04c8a2..f1641c6fe203 100644
>> > > > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > > > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>> > > > @@ -1662,6 +1662,12 @@ config {
>> > > >  			};
>> > > >  		};
>> > > >
>> > > > +		watchdog@17c10000 {
>> > > > +			compatible = "qcom,apss-wdt-sm8250", "qcom,kpss-wdt";
>> > >
>> > > Need to add this compatible to bindings.
>> > >
>> >
>> > I did look into this but the binding says,
>> > "compatible : shall contain only one of the following"
>> >
>> > So clearly the fallback is not going to work and there is no need to add
>> > a
>> > dedicated compatible in the driver. The binding is not in the YAML
>> > format to
>> > be validated but still... Other option is to convert the binding to YAML
>> > and
>> > make the compatibles listed as 'enum' and 'const' elements
>> > appropriately.
>> >
>> 
>> I already converted the bindings to YAML and added the missing 
>> compatibles
>> for
>> other SoCs[1] .
> 
> Ah, okay. I didn't find it in linux-next. Anyway, for adding
> "qcom,apss-wdt-sm8250" compatible, we need to group the compatibles 
> wisely
> using 'enum' and 'const'.
> 

Yes.

>> I thought it was already merged since Rob has already
>> reviewed
>> them, but seems like it was missed. Bjorn, can you please take it?
>> 
> 
> Perhaps for v5.9...
> 

Sure, no problem.

Thanks,
Sai
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/7] arm64: dts: qcom: pm8009: Add base dts file

[Manivannan Sadhasivam]

On Thu, Jun 04, 2020 at 03:43:25AM +0300, Dmitry Baryshkov wrote:
> Add base DTS file for pm8009 along with GPIOs and power-on nodes.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/pm8009.dtsi | 40 ++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/pm8009.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm8009.dtsi b/arch/arm64/boot/dts/qcom/pm8009.dtsi
> new file mode 100644
> index 000000000000..9f3e19b5bd00
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pm8009.dtsi
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2019, Linaro Limited

2020

> + */
> +
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/spmi/spmi.h>
> +#include <dt-bindings/iio/qcom,spmi-vadc.h>

Sort includes alphabetically.

Thanks,
Mani

> +
> +&spmi_bus {
> +	pmic@a {
> +		compatible = "qcom,pm8009", "qcom,spmi-pmic";
> +		reg = <0xa SPMI_USID>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		power-on@800 {
> +			compatible = "qcom,pm8916-pon";
> +			reg = <0x0800>;
> +		};
> +
> +		pm8009_gpios: gpio@c000 {
> +			compatible = "qcom,pm8005-gpio";
> +			reg = <0xc000>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +		};
> +	};
> +
> +	pmic@b {
> +		compatible = "qcom,pm8009", "qcom,spmi-pmic";
> +		reg = <0xb SPMI_USID>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +	};
> +};
> -- 
> 2.26.2
> 

Re: [PATCH 1/7] arm64: dts: qcom: pm8009: Add base dts file

[Vinod Koul]

On 04-06-20, 03:43, Dmitry Baryshkov wrote:
> Add base DTS file for pm8009 along with GPIOs and power-on nodes.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/pm8009.dtsi | 40 ++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/pm8009.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm8009.dtsi b/arch/arm64/boot/dts/qcom/pm8009.dtsi
> new file mode 100644
> index 000000000000..9f3e19b5bd00
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pm8009.dtsi
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2019, Linaro Limited

We are in 2020 :)

> + */
> +
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>

DO you need these headers for base patch, we should add them when adding
uses of this file

> +#include <dt-bindings/spmi/spmi.h>
> +#include <dt-bindings/iio/qcom,spmi-vadc.h>

This one too

> +
> +&spmi_bus {
> +	pmic@a {
> +		compatible = "qcom,pm8009", "qcom,spmi-pmic";
> +		reg = <0xa SPMI_USID>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		power-on@800 {
> +			compatible = "qcom,pm8916-pon";
> +			reg = <0x0800>;
> +		};
> +
> +		pm8009_gpios: gpio@c000 {
> +			compatible = "qcom,pm8005-gpio";
> +			reg = <0xc000>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +		};
> +	};
> +
> +	pmic@b {
> +		compatible = "qcom,pm8009", "qcom,spmi-pmic";
> +		reg = <0xb SPMI_USID>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +	};
> +};
> -- 
> 2.26.2

-- 
~Vinod

Re: [PATCH 1/7] arm64: dts: qcom: pm8009: Add base dts file

[Bjorn Andersson]

On Wed 03 Jun 17:43 PDT 2020, Dmitry Baryshkov wrote:

> Add base DTS file for pm8009 along with GPIOs and power-on nodes.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/pm8009.dtsi | 40 ++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/pm8009.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm8009.dtsi b/arch/arm64/boot/dts/qcom/pm8009.dtsi
> new file mode 100644
> index 000000000000..9f3e19b5bd00
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pm8009.dtsi
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2019, Linaro Limited
> + */
> +
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/spmi/spmi.h>
> +#include <dt-bindings/iio/qcom,spmi-vadc.h>
> +
> +&spmi_bus {
> +	pmic@a {
> +		compatible = "qcom,pm8009", "qcom,spmi-pmic";
> +		reg = <0xa SPMI_USID>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		power-on@800 {

Please name this "pon", just for consistency.

> +			compatible = "qcom,pm8916-pon";

Can you confirm that pm8009 is subtype 1, and hence the reboot reason
going into RB_SPARE should be shifted 2 steps, rather than 1 as in the
newer PMICs?

Would be nice if we had a generic compatible that read the subtype,
rather than having to add new compatibles for each pmic (or use the old
ones)...

Regards,
Bjorn

> +			reg = <0x0800>;
> +		};
> +
> +		pm8009_gpios: gpio@c000 {
> +			compatible = "qcom,pm8005-gpio";
> +			reg = <0xc000>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +		};
> +	};
> +
> +	pmic@b {
> +		compatible = "qcom,pm8009", "qcom,spmi-pmic";
> +		reg = <0xb SPMI_USID>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +	};
> +};
> -- 
> 2.26.2
> 

Re: [PATCH 1/7] arm64: dts: qcom: pm8009: Add base dts file

[Dmitry Baryshkov]

On Sun, 21 Jun 2020 at 09:45, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Wed 03 Jun 17:43 PDT 2020, Dmitry Baryshkov wrote:
>
> > Add base DTS file for pm8009 along with GPIOs and power-on nodes.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/pm8009.dtsi | 40 ++++++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/qcom/pm8009.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/qcom/pm8009.dtsi b/arch/arm64/boot/dts/qcom/pm8009.dtsi
> > new file mode 100644
> > index 000000000000..9f3e19b5bd00
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/pm8009.dtsi
> > @@ -0,0 +1,40 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
> > + * Copyright (c) 2019, Linaro Limited
> > + */
> > +
> > +#include <dt-bindings/input/input.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/spmi/spmi.h>
> > +#include <dt-bindings/iio/qcom,spmi-vadc.h>
> > +
> > +&spmi_bus {
> > +     pmic@a {
> > +             compatible = "qcom,pm8009", "qcom,spmi-pmic";
> > +             reg = <0xa SPMI_USID>;
> > +             #address-cells = <1>;
> > +             #size-cells = <0>;
> > +
> > +             power-on@800 {
>
> Please name this "pon", just for consistency.
>
> > +                     compatible = "qcom,pm8916-pon";
>
> Can you confirm that pm8009 is subtype 1, and hence the reboot reason
> going into RB_SPARE should be shifted 2 steps, rather than 1 as in the
> newer PMICs?

For pm8009 the subtype reads as 0x07, while according to msm-4.19 gen2
handling is only enabled for subtypes 0x04 and 0x05.

> Would be nice if we had a generic compatible that read the subtype,
> rather than having to add new compatibles for each pmic (or use the old
> ones)...

Will do it in a separate patch.

-- 
With best wishes
Dmitry