| Message ID | 20250611-ipq5018-tsens-v11-1-266566bfd16a@outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for IPQ5018 tsens | expand |
Hi Krzysztof, On 6/11/25 10:51, Krzysztof Kozlowski wrote: > On 11/06/2025 07:12, George Moussalem via B4 Relay wrote: >> From: George Moussalem <george.moussalem@outlook.com> >> >> IPQ5018 tsens should not use qcom,tsens-v1 as fallback since it has no RPM >> and, as such, deviates from the standard v1 init routine in the driver. >> So let's make qcom,ipq5018-tsens a standalone compatible in the bindings. >> >> Signed-off-by: George Moussalem <george.moussalem@outlook.com> >> --- >> Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 5 ++++- > > You just added it recently with the fallback (in v9 of this patchset) > and now remove it? > > And what does it mean it has no RPM? How does it affect the driver? Does > fallback work or not? IPQ5018 tsens IP is V1, but since it's got no RPM, it follows a different init routine for which VER_1_X_NO_RPM was created just like there is one for V2 without RPM, else the driver wouldn't probe. This was added as part of: https://patchwork.kernel.org/project/linux-arm-msm/patch/DS7PR19MB8883C5D7974C7735E23923769DCC2@DS7PR19MB8883.namprd19.prod.outlook.com/ Since its introduction, I missed updating the bindings which caused a binding issue (as reported by Rob) on the compatible as it expects the qcom,tsens-v1 as a fallback. But we can't use that fallback, so that's why it needs to be a standalone compatible. > > >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >> index 0e653bbe9884953b58c4d8569b8d096db47fd54f..73d722bda8adc2c930edfc3373e6011f19c7c491 100644 >> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >> @@ -36,10 +36,13 @@ properties: >> - qcom,msm8974-tsens >> - const: qcom,tsens-v0_1 >> >> + - description: v1 of TSENS > > So that's still v1... I don't understand. As mentioned, the IP is still v1 but with a different init routine in the driver for IP v1 without RPM > >> + enum: >> + - qcom,ipq5018-tsens >> + >> - description: v1 of TSENS >> items: >> - enum: >> - - qcom,ipq5018-tsens >> - qcom,msm8937-tsens >> - qcom,msm8956-tsens >> - qcom,msm8976-tsens >> > > > Best regards, > Krzysztof Best regards, George
On 6/11/25 11:13, Krzysztof Kozlowski wrote: > On 11/06/2025 09:06, George Moussalem wrote: >> Hi Krzysztof, >> >> On 6/11/25 10:51, Krzysztof Kozlowski wrote: >>> On 11/06/2025 07:12, George Moussalem via B4 Relay wrote: >>>> From: George Moussalem <george.moussalem@outlook.com> >>>> >>>> IPQ5018 tsens should not use qcom,tsens-v1 as fallback since it has no RPM >>>> and, as such, deviates from the standard v1 init routine in the driver. >>>> So let's make qcom,ipq5018-tsens a standalone compatible in the bindings. >>>> >>>> Signed-off-by: George Moussalem <george.moussalem@outlook.com> >>>> --- >>>> Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 5 ++++- >>> >>> You just added it recently with the fallback (in v9 of this patchset) >>> and now remove it? >>> >>> And what does it mean it has no RPM? How does it affect the driver? Does >>> fallback work or not? >> >> IPQ5018 tsens IP is V1, but since it's got no RPM, it follows a >> different init routine for which VER_1_X_NO_RPM was created just like >> there is one for V2 without RPM, else the driver wouldn't probe. This >> was added as part of: >> >> https://patchwork.kernel.org/project/linux-arm-msm/patch/DS7PR19MB8883C5D7974C7735E23923769DCC2@DS7PR19MB8883.namprd19.prod.outlook.com/ >> >> Since its introduction, I missed updating the bindings which caused a >> binding issue (as reported by Rob) on the compatible as it expects the >> qcom,tsens-v1 as a fallback. But we can't use that fallback, so that's >> why it needs to be a standalone compatible. > > > So you need Fixes tag and explain what was buggy in previous patch. will add, thanks. > >> >>> >>> >>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >>>> index 0e653bbe9884953b58c4d8569b8d096db47fd54f..73d722bda8adc2c930edfc3373e6011f19c7c491 100644 >>>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >>>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml >>>> @@ -36,10 +36,13 @@ properties: >>>> - qcom,msm8974-tsens >>>> - const: qcom,tsens-v0_1 >>>> >>>> + - description: v1 of TSENS >>> >>> So that's still v1... I don't understand. >> >> As mentioned, the IP is still v1 but with a different init routine in >> the driver for IP v1 without RPM > > OK, just merge it into first enum and drop the description there. can't merge it into the first enum as that description is invalid for this SoC ("description: msm8960 TSENS based"). My proposal would be: - description: v1 of TSENS oneOf: - enum: # for IP V1 without RPM - qcom,ipq5018-tsens - items: - enum: - qcom,msm8937-tsens - qcom,msm8956-tsens - qcom,msm8976-tsens - qcom,qcs404-tsens - const: qcom,tsens-v1 thoughts? > > Best regards, > Krzysztof Best regards, George
diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml index 0e653bbe9884953b58c4d8569b8d096db47fd54f..73d722bda8adc2c930edfc3373e6011f19c7c491 100644 --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml @@ -36,10 +36,13 @@ properties: - qcom,msm8974-tsens - const: qcom,tsens-v0_1 + - description: v1 of TSENS + enum: + - qcom,ipq5018-tsens + - description: v1 of TSENS items: - enum: - - qcom,ipq5018-tsens - qcom,msm8937-tsens - qcom,msm8956-tsens - qcom,msm8976-tsens