Re: [PATCH 01/11] dt-bindings: remoteproc: qcom: Add support for multipd model

[Manikanta Mylavarapu]

On 5/9/2023 12:38 PM, Krzysztof Kozlowski wrote:
> On 08/05/2023 15:45, Manikanta Mylavarapu wrote:
> > On 3/7/2023 8:47 PM, Krzysztof Kozlowski wrote:
> > > On 07/03/2023 05:41, Manikanta Mylavarapu wrote:
> > > > Add new binding document for multipd model remoteproc.
> > > > IPQ5018, IPQ9574 follows multipd model.
> > > >
> > > > Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@xxxxxxxxxxx>
> > > > ---
> > > >    .../bindings/remoteproc/qcom,multipd-pil.yaml | 282 ++++++++++++++++++
> > > >    1 file changed, 282 insertions(+)
> > > >    create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml
> > > > new file mode 100644
> > > > index 000000000000..b788607f5abd
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,multipd-pil.yaml
> > > > @@ -0,0 +1,282 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/remoteproc/qcom,multipd-pil.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Qualcomm Multipd Secure Peripheral Image Loader
> > > > +
> > > > +maintainers:
> > > > +  - Bjorn Andersson <andersson@xxxxxxxxxx>
> > > > +  - Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> > > > +
> > > > +description:
> > > > +  Multipd Peripheral Image Loader loads firmware and boots Q6 pd, WCSS pd
> > > > +  remoteproc's on the Qualcomm IPQ5018, IPQ9574 SoC.
> > >
> > > What is a "pd"?
> > Pd means protection domain.
> > It's similar to process in Linux. Here QDSP6 processor runs each wifi
> > radio functionality on a separate process. One process can't access
> > other process resources, so this is termed as PD i.e protection domain.
> > Here we have two pd's called root and user pd. We can correlate Root pd
> > as root and user pd as user in linux. Root pd has more privileges than
> > user pd.
> >   From remoteproc driver perspective, root pd corresponds to QDSP6
> > processor bring up and user pd corresponds to Wifi radio (WCSS) bring up.
>
> Parts of it should be in description. And definitely "pd" should be
> explained.
Sure, i will add it in cover page.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - qcom,ipq5018-q6-mpd
> > > > +      - qcom,ipq9574-q6-mpd
> > > > +
> > > > +  '#address-cells': true
> > > > +
> > > > +  '#size-cells': true
>
> ...
>
> > >
> > Sure, will add.
> > > > +    description:
> > > > +      Qualcomm G-Link subnode which represents communication edge, channels
> > > > +      and devices related to the Modem.
> > > > +
> > > > +patternProperties:
> > > > +  "^remoteproc_pd1|remoteproc_pd2|remoteproc_pd3":
> > >
> > > No, underscores are not allowed. Also, what is pd?
> > Sure, will remove underscores.
>
> Shouldn't this be just pd-1?
I think 'pd-1' not enough. Because child's i.e userpd's also considered
as remote processor's, so name should be like "remoteproc-pd1".
> > > > +    type: object
> > > > +    description:
> > > > +      In Multipd model, WCSS pd depends on Q6 pd i.e Q6 pd should be up before
> > > > +      WCSS. It can be achieved by keeping wcss pd node as subnode of Q6
> > > > +      device node.
> > > > +
> > > > +    properties:
> > > > +      compatible:
> > > > +        enum:
> > > > +          - "qcom,ipq5018-wcss-ahb-mpd"
> > > > +          - "qcom,ipq9574-wcss-ahb-mpd"
> > > > +          - "qcom,ipq5018-wcss-pcie-mpd"
>
>
>
> Best regards,
> Krzysztof

Thanks & Regards,
Manikanta.