Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d187eafb-4a80-9479-d063-3a01b47d8efa@linaro.org>
Date:   Tue, 6 Jun 2023 08:14:33 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.2
Subject: Re: [PATCH V2 01/13] dt-bindings: remoteproc: qcom: Add support for
 multipd model
Content-Language: en-US
To:     Manikanta Mylavarapu <quic_mmanikan@quicinc.com>,
        agross@kernel.org, andersson@kernel.org, konrad.dybcio@linaro.org,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        conor+dt@kernel.org, jassisinghbrar@gmail.com,
        mathieu.poirier@linaro.org, mturquette@baylibre.com,
        sboyd@kernel.org, quic_eberman@quicinc.com, quic_mojha@quicinc.com,
        kvalo@kernel.org, loic.poulain@linaro.org,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-remoteproc@vger.kernel.org,
        linux-clk@vger.kernel.org
Cc:     quic_srichara@quicinc.com, quic_sjaganat@quicinc.com,
        quic_kathirav@quicinc.com, quic_anusha@quicinc.com,
        quic_poovendh@quicinc.com, quic_varada@quicinc.com,
        quic_devipriy@quicinc.com
References: <20230521222852.5740-1-quic_mmanikan@quicinc.com>
 <20230521222852.5740-2-quic_mmanikan@quicinc.com>
 <7940c743-815f-f864-d015-43d7e916ecfa@linaro.org>
 <a1456f62-d0a7-d5ec-b379-db1b6035c89c@quicinc.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <a1456f62-d0a7-d5ec-b379-db1b6035c89c@quicinc.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 05/06/2023 14:02, Manikanta Mylavarapu wrote:
>>> +  memory-region:
>>> +    items:
>>> +      - description: Q6 pd reserved region
>>> +
>>> +  glink-edge:
>>> +    $ref: /schemas/remoteproc/qcom,glink-edge.yaml#
>>> +    description:
>>> +      Qualcomm G-Link subnode which represents communication edge, channels
>>> +      and devices related to the Modem.
>>> +
>>> +patternProperties:
>>> +  "^pd-1|pd-2|pd-3":
>>> +    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.
>>
>> That's not enough. Your description does not say what is this, why you
>> have two protection domains for same compatible. What's more, it a bit
>> deviates from hardware description.
>>
> WCSS means 'wireless connectivity sub system', in simple words it's a
> wifi radio block.
> 
> IPQ5018 SOC has both internal (AHB) wifi radio/WCSS and external (PCIE)
> wifi radio/WCSS. In Q6, Root protection domain will provide services to
> both internal (AHB) and external (PCIE) wifi radio's protection domain.
> So we have two protection domains for IPQ5018, one is for internal(AHB) 
> and other is for external(PCIE) wifi radio.

So it is now in email, but not in the code...
> 
>>> +
>>> +    properties:
>>> +      compatible:
>>> +        enum:
>>> +          - qcom,ipq5018-wcss-ahb-mpd
>>> +          - qcom,ipq9574-wcss-ahb-mpd
>>> +          - qcom,ipq5018-wcss-pcie-mpd
>>
>> Keep rather alphabetical order (so both 5018 together).
>>
>> I also do not understand these at all. Why adding bus type to
>> compatible? This rarely is allowed (unless it is PCIe controller within
>> soc).
>>
> IPQ5018 SOC has in-built PCIE controller. Here QDSP6 will bring up
> external(PCIE) and internal (AHB) wifi radio's. To separate AHB, PCIE 
> radio's properties, i have added bus type to compatible.

It's the same device - WCSS - right? We do not create multiple nodes and
compatibles for the same devices. Bus suffixes are almost never parts of
compatibles.


>>
>> Drop.
>>
>>> +
>>> +unevaluatedProperties: false
>>
>> This changed... why?
>>
>>
> 'unevaluatedProperties' is similar to 'additionalProperties' except
> that it recognize properties declared in subschemas as well.

You don't have to explain me what are unevaluatedProperties or
additionalProperties. Let's assume that I know them. What you should
explain is why you changed it. Where is the reference to other schema?


Best regards,
Krzysztof