mbox series

[V3,0/4] Add support for IPQ5018 tsens

Message ID 20230922115116.2748804-1-srichara@win-platform-upstream01.qualcomm.com
Headers show
Series Add support for IPQ5018 tsens | expand

Message

Sricharan R Sept. 22, 2023, 11:51 a.m. UTC
IPQ5018 has tsens V1.0 IP with 4 sensors and 1 interrupt.
There is no RPM present in the soc to do tsens early enable.
Adding support for the same here.

[v3]
	*) Added the tsens-ipq5018 as  new binding without rpm
        *) Added Dmitry's Reviewed tag
        *) Fixed Dmitry's comments for error checks in init_ipq5018
        *) Ordered the qfprom device node properties

[v2]
	*) Sorted the compatible and removed example
	*) Fixed the name for new tsens_feature
	*) Used tsend_calibrate_common instead of legacy
	   and addressed comments from Dmitry.
	*) Squashed patch 3 & 4
	*) Fixed node names, order and added qfprom cells
            for points seprately
	*) Squashed patch 6 & 7 

Sricharan Ramabadhran (4):
  dt-bindings: thermal: qcom-tsens: Add ipq5018 compatible
  thermal/drivers/qcom: Add new feat for soc without rpm
  thermal/drivers/tsens: Add support for IPQ5018 tsens
  arm64: dts: qcom: ipq5018: Add tsens node

 .../bindings/thermal/qcom-tsens.yaml          |   4 +
 arch/arm64/boot/dts/qcom/ipq5018.dtsi         | 169 ++++++++++++++++++
 drivers/thermal/qcom/tsens-v1.c               |  60 +++++++
 drivers/thermal/qcom/tsens.c                  |   5 +-
 drivers/thermal/qcom/tsens.h                  |   5 +-
 5 files changed, 241 insertions(+), 2 deletions(-)

Comments

Krzysztof Kozlowski Sept. 23, 2023, 11:44 a.m. UTC | #1
On 22/09/2023 13:51, Sricharan R wrote:
> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> 
> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.

Then why do you allow two interrupts?

> 
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> ---
>  [v3] Added the tsens-ipq5018 as  new binding without rpm
> 
>  Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> index 27e9e16e6455..a02829deeb24 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> @@ -44,6 +44,10 @@ properties:
>                - qcom,qcs404-tsens
>            - const: qcom,tsens-v1
>  
> +      - description: v1 of TSENS without rpm
> +        enum:
> +          - qcom,ipq5018-tsens

You miss now description of interrupts, like the other variants have.

Best regards,
Krzysztof
Dmitry Baryshkov Sept. 23, 2023, 6:48 p.m. UTC | #2
On Fri, 22 Sept 2023 at 14:51, Sricharan R
<srichara@win-platform-upstream01.qualcomm.com> wrote:
>
> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>
> IPQ5018 has tsens V1.0 IP with 4 sensors.
> There is no RPM, so tsens has to be manually enabled. Adding the tsens
> and nvmem node and IPQ5018 has 4 thermal sensors (zones). With the
> critical temperature being 120'C and action is to reboot. Adding all
> the 4 zones here.
>
> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
> ---
>  [v3] Ordered the qfprom device node properties as per
>       Krzysztof's comments
>
>  arch/arm64/boot/dts/qcom/ipq5018.dtsi | 169 ++++++++++++++++++++++++++
>  1 file changed, 169 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
> index 9f13d2dcdfd5..9e28b54ebcbd 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
> @@ -93,6 +93,117 @@ soc: soc@0 {
>                 #size-cells = <1>;
>                 ranges = <0 0 0 0xffffffff>;
>
> +               qfprom: qfprom@a0000 {
> +                       compatible = "qcom,ipq5018-qfprom", "qcom,qfprom";
> +                       reg = <0xa0000 0x1000>;
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +
> +                       tsens_base1: base1@249 {
> +                               reg = <0x249 2>;
> +                               bits = <3 8>;
> +                       };
> +
> +                       tsens_base2: base2@24a {
> +                               reg = <0x24a 2>;
> +                               bits = <3 8>;
> +                       };

Sort by the address, please, as usual.

> +
> +                       tsens_mode: mode@249 {
> +                               reg = <0x249 1>;
> +                               bits = <0 3>;
> +                       };
> +
> +                       tsens_s0_p1: s0-p1@24b {
> +                               reg = <0x24b 0x2>;
> +                               bits = <2 6>;
> +                       };
> +
> +                       tsens_s0_p2: s0-p2@24c {
> +                               reg = <0x24c 0x1>;
> +                               bits = <1 6>;
> +                       };
> +
> +                       tsens_s1_p1: s1-p1@24c {
> +                               reg = <0x24c 0x2>;
> +                               bits = <7 6>;
> +                       };
> +
> +                       tsens_s1_p2: s1-p2@24d {
> +                               reg = <0x24d 0x2>;
> +                               bits = <5 6>;
> +                       };
> +
> +                       tsens_s2_p1: s2-p1@24e {
> +                               reg = <0x24e 0x2>;
> +                               bits = <3 6>;
> +                       };
> +
> +                       tsens_s2_p2: s2-p2@24f {
> +                               reg = <0x24f 0x1>;
> +                               bits = <1 6>;
> +                       };
> +
> +                       tsens_s3_p1: s3-p1@24f {
> +                               reg = <0x24f 0x2>;
> +                               bits = <7 6>;
> +                       };
> +
> +                       tsens_s3_p2: s3-p2@250 {
> +                               reg = <0x250 0x2>;
> +                               bits = <5 6>;
> +                       };
> +
> +                       tsens_s4_p1: s4-p1@251 {
> +                               reg = <0x251 0x2>;
> +                               bits = <3 6>;
> +                       };
> +
> +                       tsens_s4_p2: s4-p2@254 {
> +                               reg = <0x254 0x1>;
> +                               bits = <0 6>;
> +                       };
> +               };
> +
> +               tsens: thermal-sensor@4a9000 {
> +                       compatible = "qcom,ipq5018-tsens";
> +                       reg = <0x4a9000 0x1000>, /* TM */
> +                             <0x4a8000 0x1000>; /* SORT */
> +
> +                       nvmem-cells = <&tsens_mode>,
> +                                     <&tsens_base1>,
> +                                     <&tsens_base2>,
> +                                     <&tsens_s0_p1>,
> +                                     <&tsens_s0_p2>,
> +                                     <&tsens_s1_p1>,
> +                                     <&tsens_s1_p2>,
> +                                     <&tsens_s2_p1>,
> +                                     <&tsens_s2_p2>,
> +                                     <&tsens_s3_p1>,
> +                                     <&tsens_s3_p2>,
> +                                     <&tsens_s4_p1>,
> +                                     <&tsens_s4_p2>;
> +
> +                       nvmem-cell-names = "mode",
> +                                          "base1",
> +                                          "base2",
> +                                          "s0_p1",
> +                                          "s0_p2",
> +                                          "s1_p1",
> +                                          "s1_p2",
> +                                          "s2_p1",
> +                                          "s2_p2",
> +                                          "s3_p1",
> +                                          "s3_p2",
> +                                          "s4_p1",
> +                                          "s4_p2";
> +
> +                       interrupts = <GIC_SPI 184 IRQ_TYPE_EDGE_RISING>;
> +                       interrupt-names = "uplow";
> +                       #qcom,sensors = <5>;
> +                       #thermal-sensor-cells = <1>;
> +               };
> +
>                 tlmm: pinctrl@1000000 {
>                         compatible = "qcom,ipq5018-tlmm";
>                         reg = <0x01000000 0x300000>;
> @@ -240,6 +351,64 @@ frame@b128000 {
>                 };
>         };
>
> +       thermal-zones {
> +               ubi32-thermal {
> +                       polling-delay-passive = <0>;
> +                       polling-delay = <0>;
> +                       thermal-sensors = <&tsens 1>;
> +
> +                       trips {
> +                               ubi32-critical {
> +                                       temperature = <120000>;
> +                                       hysteresis = <2>;
> +                                       type = "critical";
> +                               };
> +                       };
> +               };
> +
> +               cpu-thermal {

'c' < 'u'. Please sort the nodes here.

> +                       polling-delay-passive = <0>;
> +                       polling-delay = <0>;
> +                       thermal-sensors = <&tsens 2>;
> +
> +                       trips {
> +                               cpu-critical {
> +                                       temperature = <120000>;
> +                                       hysteresis = <2>;
> +                                       type = "critical";
> +                               };
> +                       };
> +               };
> +
> +               top-glue-thermal {
> +                       polling-delay-passive = <0>;
> +                       polling-delay = <0>;
> +                       thermal-sensors = <&tsens 3>;
> +
> +                       trips {
> +                               top_glue-critical {
> +                                       temperature = <120000>;
> +                                       hysteresis = <2>;
> +                                       type = "critical";
> +                               };
> +                       };
> +               };
> +
> +               gephy-thermal {
> +                       polling-delay-passive = <0>;
> +                       polling-delay = <0>;
> +                       thermal-sensors = <&tsens 4>;
> +
> +                       trips {
> +                               gephy-critical {
> +                                       temperature = <120000>;
> +                                       hysteresis = <2>;
> +                                       type = "critical";
> +                               };
> +                       };
> +               };
> +       };
> +
>         timer {
>                 compatible = "arm,armv8-timer";
>                 interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> --
> 2.34.1
>
Sricharan Ramabadhran Sept. 25, 2023, 2:06 a.m. UTC | #3
On 9/23/2023 5:14 PM, Krzysztof Kozlowski wrote:
> On 22/09/2023 13:51, Sricharan R wrote:
>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>
>> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.
> 
> Then why do you allow two interrupts?
> 
  infact there is only one interrupt. Will fix in the binding
  description.

>>
>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>> ---
>>   [v3] Added the tsens-ipq5018 as  new binding without rpm
>>
>>   Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>> index 27e9e16e6455..a02829deeb24 100644
>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
>> @@ -44,6 +44,10 @@ properties:
>>                 - qcom,qcs404-tsens
>>             - const: qcom,tsens-v1
>>   
>> +      - description: v1 of TSENS without rpm
>> +        enum:
>> +          - qcom,ipq5018-tsens
> 
> You miss now description of interrupts, like the other variants have.
> 

  ok, will add it.

Regards,
  Sricharan
Sricharan Ramabadhran Sept. 25, 2023, 2:09 a.m. UTC | #4
On 9/24/2023 12:18 AM, Dmitry Baryshkov wrote:
> On Fri, 22 Sept 2023 at 14:51, Sricharan R
> <srichara@win-platform-upstream01.qualcomm.com> wrote:
>>
>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>
>> IPQ5018 has tsens V1.0 IP with 4 sensors.
>> There is no RPM, so tsens has to be manually enabled. Adding the tsens
>> and nvmem node and IPQ5018 has 4 thermal sensors (zones). With the
>> critical temperature being 120'C and action is to reboot. Adding all
>> the 4 zones here.
>>
>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>> ---
>>   [v3] Ordered the qfprom device node properties as per
>>        Krzysztof's comments
>>
>>   arch/arm64/boot/dts/qcom/ipq5018.dtsi | 169 ++++++++++++++++++++++++++
>>   1 file changed, 169 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>> index 9f13d2dcdfd5..9e28b54ebcbd 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
>> @@ -93,6 +93,117 @@ soc: soc@0 {
>>                  #size-cells = <1>;
>>                  ranges = <0 0 0 0xffffffff>;
>>
>> +               qfprom: qfprom@a0000 {
>> +                       compatible = "qcom,ipq5018-qfprom", "qcom,qfprom";
>> +                       reg = <0xa0000 0x1000>;
>> +                       #address-cells = <1>;
>> +                       #size-cells = <1>;
>> +
>> +                       tsens_base1: base1@249 {
>> +                               reg = <0x249 2>;
>> +                               bits = <3 8>;
>> +                       };
>> +
>> +                       tsens_base2: base2@24a {
>> +                               reg = <0x24a 2>;
>> +                               bits = <3 8>;
>> +                       };
> 
> Sort by the address, please, as usual.
> 

  ok.

<..>

>>
>> +       thermal-zones {
>> +               ubi32-thermal {
>> +                       polling-delay-passive = <0>;
>> +                       polling-delay = <0>;
>> +                       thermal-sensors = <&tsens 1>;
>> +
>> +                       trips {
>> +                               ubi32-critical {
>> +                                       temperature = <120000>;
>> +                                       hysteresis = <2>;
>> +                                       type = "critical";
>> +                               };
>> +                       };
>> +               };
>> +
>> +               cpu-thermal {
> 
> 'c' < 'u'. Please sort the nodes here.

  ok

Regards,
  Sricharan
Krzysztof Kozlowski Sept. 25, 2023, 6:46 a.m. UTC | #5
On 25/09/2023 04:06, Sricharan Ramabadhran wrote:
> 
> 
> On 9/23/2023 5:14 PM, Krzysztof Kozlowski wrote:
>> On 22/09/2023 13:51, Sricharan R wrote:
>>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>
>>> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.
>>
>> Then why do you allow two interrupts?
>>
>   infact there is only one interrupt. Will fix in the binding
>   description.

Description? So you still allow two interrupts? No, this must be
constrained in allOf:if:then.

Best regards,
Krzysztof
Sricharan Ramabadhran Sept. 25, 2023, 10:31 a.m. UTC | #6
On 9/25/2023 12:16 PM, Krzysztof Kozlowski wrote:
> On 25/09/2023 04:06, Sricharan Ramabadhran wrote:
>>
>>
>> On 9/23/2023 5:14 PM, Krzysztof Kozlowski wrote:
>>> On 22/09/2023 13:51, Sricharan R wrote:
>>>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>>
>>>> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.
>>>
>>> Then why do you allow two interrupts?
>>>
>>    infact there is only one interrupt. Will fix in the binding
>>    description.
> 
> Description? So you still allow two interrupts? No, this must be
> constrained in allOf:if:then.
> 

  ok, it should be fine to add this new compatible to the existing
  allof:if:then block of v1 targets ? (Because they also have same
  single interrupt (uplow) constraint)

Regards,
  Sricharan
Krzysztof Kozlowski Sept. 25, 2023, 10:40 a.m. UTC | #7
On 25/09/2023 12:31, Sricharan Ramabadhran wrote:
> 
> 
> On 9/25/2023 12:16 PM, Krzysztof Kozlowski wrote:
>> On 25/09/2023 04:06, Sricharan Ramabadhran wrote:
>>>
>>>
>>> On 9/23/2023 5:14 PM, Krzysztof Kozlowski wrote:
>>>> On 22/09/2023 13:51, Sricharan R wrote:
>>>>> From: Sricharan Ramabadhran <quic_srichara@quicinc.com>
>>>>>
>>>>> IPQ5018 has tsens v1.0 block with 4 sensors and 1 interrupt.
>>>>
>>>> Then why do you allow two interrupts?
>>>>
>>>    infact there is only one interrupt. Will fix in the binding
>>>    description.
>>
>> Description? So you still allow two interrupts? No, this must be
>> constrained in allOf:if:then.
>>
> 
>   ok, it should be fine to add this new compatible to the existing
>   allof:if:then block of v1 targets ? (Because they also have same
>   single interrupt (uplow) constraint)

Yes.

Best regards,
Krzysztof