| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 17 May 2023 11:27:27 +0530
From: Varadarajan Narayanan <quic_varada@...cinc.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
CC: <agross@...nel.org>, <andersson@...nel.org>,
<konrad.dybcio@...aro.org>, <amitk@...nel.org>,
<thara.gopinath@...il.com>, <rafael@...nel.org>,
<daniel.lezcano@...aro.org>, <rui.zhang@...el.com>,
<robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
<conor+dt@...nel.org>, <linux-arm-msm@...r.kernel.org>,
<linux-pm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
Praveenkumar I <quic_ipkumar@...cinc.com>
Subject: Re: [PATCH v3 2/4] dt-bindings: thermal: tsens: Add ipq9574
compatible
On Tue, May 16, 2023 at 03:06:40PM +0200, Krzysztof Kozlowski wrote:
> On 16/05/2023 14:04, Varadarajan Narayanan wrote:
> > On Mon, May 15, 2023 at 06:10:29PM +0200, Krzysztof Kozlowski wrote:
> >> On 15/05/2023 12:13, Varadarajan Narayanan wrote:
> >>> From: Praveenkumar I <quic_ipkumar@...cinc.com>
> >>>
> >>> Qualcomm IPQ9574 has tsens v2.3.1 block, which is similar to IPQ8074 tsens.
> >>>
> >>> Signed-off-by: Praveenkumar I <quic_ipkumar@...cinc.com>
> >>> Signed-off-by: Varadarajan Narayanan <quic_varada@...cinc.com>
> >>> ---
> >>> [v3]:
> >>> Fix dt_binding_check & dtbs_check errors (Used
> >>> Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> >>> as reference/example)
> >>>
> >>> Drop 'Acked-by: Rob Herring' as suggested in review
> >>>
> >>> [v2]:
> >>> Thanks to Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> >>> for the tip to make qcom,ipq8074-tsens as fallback.
> >>> ---
> >>> Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 13 +++++++++++--
> >>> 1 file changed, 11 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> >>> index d9aa54c..57e3908 100644
> >>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> >>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> >>> @@ -19,6 +19,11 @@ description: |
> >>> properties:
> >>> compatible:
> >>> oneOf:
> >>> + - const: qcom,tsens-v0_1
> >>> + - const: qcom,tsens-v1
> >>> + - const: qcom,tsens-v2
> >>
> >> Nope, these are not correct.
> >>
> >>> + - const: qcom,ipq8074-tsens
> >>
> >> Also nope, this is already there.
> >>
> >>> +
> >>> - description: msm8960 TSENS based
> >>> items:
> >>> - enum:
> >>> @@ -66,8 +71,10 @@ properties:
> >>> - const: qcom,tsens-v2
> >>>
> >>> - description: v2 of TSENS with combined interrupt
> >>> - enum:
> >>> - - qcom,ipq8074-tsens
> >>
> >> Why?
> >>
> >>> + items:
> >>> + - enum:
> >>> + - qcom,ipq9574-tsens
> >>> + - const: qcom,ipq8074-tsens
> >
> > Without changing it like this either dtbs_check or
> > dt_binding_check kept failing.
> >
> > - description: v2 of TSENS with combined interrupt
> > enum:
> > - qcom,ipq8074-tsens
> > - qcom,ipq9574-tsens
>
> But we do not talk about this... Look, I commented out under specific
> hunks which are not correct. Not under the hunk which is correct.
>
> >
> > dtbs_check gave this kind of error
> > ['qcom,ipq9574-tsens', 'qcom,ipq8074-tsens'] is too long
> >
> > After changing it like in https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L31
> >
> > - description: v2 of TSENS with combined interrupt
> > const: qcom,ipq8074-tsens
> > - enum:
> > - qcom,ipq9574-tsens
> > - const: qcom,ipq8074-tsens
> >
> > dt_binding_check gives the following error
> >
> > Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
>
> Because it is not even valid syntax.
>
> >
> > and dtbs_check gives
> >
> > ./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: [error] syntax error: expected <block end>, but found '-' (syntax)
> > CHKDT Documentation/devicetree/bindings/processed-schema.json
> > ./Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
> > ./Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/thermal/qcom-tsens.yaml
> > ./Documentation/devicetree/bindings/thermal/qcom-tsens.yaml:70:9: did not find expected key
> > SCHEMA Documentation/devicetree/bindings/processed-schema.json
> > /local/mnt/workspace/varada/v3/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml: ignoring, error parsing file
> >
> > If i change it like below,
> >
> > - description: v2 of TSENS with combined interrupt
> > enum:
> > - qcom,ipq9574-tsens
> > - const: qcom,ipq8074-tsens
> >
> > dt_binding_check and dtbs_check gives same error as above.
> >
> > Looked around and found Documentation/devicetree/bindings/display/allwinner,sun4i-a10-tcon.yaml
> > which seemed to do something similar to what is wanted in this
> > case. Hence changed qcom-tsens.yaml similar to the allwinner yaml
> > file. After which dt_binding_check and dtbs_check passed. Please
> > let me know if there is a better way to solve this. Will go with
>
> Changing one valid syntax to another valid syntax is not related to the
> patch. If you think such change as reasonable, please split it, but to
> me it does not look justified. As for actual change, so adding new
> compatible, it's not really related to the others. Why you cannot add
> the proper list (so the only valid hunk) and that's it?
Not sure if I didn't express properly. There are two parts to this patch.
Part-1 is adding the 'const' entries at the beginning i.e.
+ - const: qcom,tsens-v0_1
+ - const: qcom,tsens-v1
+ - const: qcom,tsens-v2
+ - const: qcom,ipq8074-tsens
Part-2 is changing from one valid syntax to another i.e.
+ items:
+ - enum:
+ - qcom,ipq9574-tsens
+ - const: qcom,ipq8074-tsens
Without both of the above changes, either or both of dtbs_check
& dt_binding_check fails. So, it is not possible to just add the
"valid hunk" (part-2) alone.
If having both part-1 and part-2 in the same patch is not
acceptable, shall I split them into two patches? Please let me know.
Thanks
Varada
Powered by blists - more mailing lists