linux-kernel - Re: [PATCH 4/9] dt-bindings: remoteproc: qcom: wcnss: Convert to YAML

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <CJXTS6RN1T67.WNKK2FZKK9UB@skynet-linux>
Date:   Thu, 12 May 2022 19:12:36 +0530
From:   "Sireesh Kodali" <sireeshkodali1@...il.com>
To:     "Krzysztof Kozlowski" <krzysztof.kozlowski@...aro.org>,
        <linux-remoteproc@...r.kernel.org>
Cc:     <linux-arm-msm@...r.kernel.org>,
        <~postmarketos/upstreaming@...ts.sr.ht>,
        <bjorn.andersson@...aro.org>, <devicetree@...r.kernel.org>,
        <phone-devel@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        "Andy Gross" <agross@...nel.org>,
        "Mathieu Poirier" <mathieu.poirier@...aro.org>,
        "Rob Herring" <robh+dt@...nel.org>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@...aro.org>,
        "Loic Poulain" <loic.poulain@...aro.org>
Subject: Re: [PATCH 4/9] dt-bindings: remoteproc: qcom: wcnss: Convert to
 YAML

On Thu May 12, 2022 at 4:32 PM IST, Krzysztof Kozlowski wrote:
> On 12/05/2022 11:32, Sireesh Kodali wrote:
> >>>>> +          - enum:
> >>>>> +              - qcom,pronto-v2-pil
> >>>>> +          - enum:
> >>>>> +              - qcom,pronto
> >>>>
> >>>> This does not look correct. The fallback compatible should not change.
> >>>> What is more, it was not documented in original binding, so this should
> >>>> be done in separate patch.
> >>>>
> >>>
> >>> This was not a change to the fallback compatible. 
> >>
> >> You made it an enum, so you expect it to use different fallback for
> >> different cases.
> >>
> >>> msm8916.dtsi's wcnss
> >>> node has "qcom,pronto" as the compatible string, which is why this was
> >>> added. It is however not documented in the txt file. Is it sufficient to
> >>> add a note in the commit message, or should it be split into a separate
> >>> commit?
> >>
> >> Please split it, assuming that fallback is correct. Maybe the fallback
> >> is wrong?
> > 
> > The code doesn't recognize "qcom,pronto", so perhaps the best solution
> > is to just remove that compatible from msm8916.dtsi?
>
> Eh, I don't know. You need to check, maybe also in downstream sources.
>

I just checked, it seems "qcom,pronto" is used by the wcnss driver in
/net. So both "qcom,pronto-v2-pil" and "qcom,pronto" need to be present,
but the latter wasn't documented.

> (...)
>
> >>>>
> >>>>> +
> >>>>> +  iris:
> >>>>
> >>>> Generic node name... what is "iris"?
> >>>>
> >>> Iris is the RF module, I'll make the description better
> >>
> >> RF like wifi? Then the property name should be "wifi".
> > 
> > RF like wifi and bluetooth. However there are wifi and bt subnodes in
> > the smd-edge subnode. Iris is just the antenna hardware if I understand
> > correctly. Also this is just a documentation of the existing nodes that
> > are present in msm8916.dtsi, but for whatever reason their documentation
> > was missing in the txt file. Without adding this node in the YAML
> > dtb_check fails.
>
> It seems commit fd52bdae9ab0 ("wcn36xx: Disable 5GHz for wcn3620")
>  added usage of "iris" property but did not document it in the bindings.
>
> You can fix it by documenting (separate patch) existing practice or
> document with changing the node name. I am not sure if it is worth the
> effort, so just new patch please.
>

I'll make a 2 separate patches, documenting the extra "qcom,pronto"
compatible, and the iris subnode.

Thanks,
Sireesh

> Best regards,
> Krzysztof