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: <feb0d11d-0930-d0b8-ab6e-cf477bbf114b@quicinc.com>
Date:   Tue, 6 Jun 2023 17:41:00 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.2
Subject: Re: [PATCH V2 01/13] dt-bindings: remoteproc: qcom: Add support for
 multipd model
Content-Language: en-US
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        <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>
 <d187eafb-4a80-9479-d063-3a01b47d8efa@linaro.org>
From:   Manikanta Mylavarapu <quic_mmanikan@quicinc.com>
In-Reply-To: <d187eafb-4a80-9479-d063-3a01b47d8efa@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 6/6/2023 11:44 AM, Krzysztof Kozlowski wrote:
> 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...
>>
I will add this info, corresponding block diagram in driver 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.
> 
> 
No it's not the same device. WCSS on inside IPQ5018 and WCSS attached 
via pcie to IPQ5018. Here QDSP6 managing both WCSS's.

So for better clarity i will use attached SOC ID in compatible.
Below are the new compatible's.

- qcom,ipq5018-wcss-mpd //IPQ5018 internal radio
- qcom,ipq9574-wcss-mpd	//IPQ9574 internal radio
- qcom,qcn6122-wcss-mpd //IPQ5018 attached radio

>>>
>>> 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?
> 
I will go with previously used 'additionalProperties' itself and add
'unevaluatedProperties' in glink-edge.

Thanks & Regards,
Manikanta.
> 
> Best regards,
> Krzysztof
>