Message ID | 20230922115116.2748804-1-srichara@win-platform-upstream01.qualcomm.com |
---|---|
Headers | show |
Series | Add support for IPQ5018 tsens | expand |
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
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 >
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
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
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
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
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