Received-SPF: pass (google.com: domain of linux-kernel+bounces-94194-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <b2ae44ff-55e2-495b-b320-cbb8deae81be@quicinc.com>
Date: Wed, 6 Mar 2024 21:19:34 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC 1/3] dt-bindings: usb: qcom,dwc3: Add support for multiple
 power-domains
Content-Language: en-US
To: Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>,
        Krzysztof Kozlowski
	<krzysztof.kozlowski@linaro.org>,
        <andersson@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
        <dmitry.baryshkov@linaro.org>
CC: <quic_shazhuss@quicinc.com>, <linux-usb@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <Thinh.Nguyen@synopsys.com>,
        <kishon@kernel.org>, <konrad.dybcio@linaro.org>, <robh@kernel.org>,
        <sudeep.holla@arm.com>, <p.zabel@pengutronix.de>,
        <ulf.hansson@linaro.org>, <conor+dt@kernel.org>,
        <manivannan.sadhasivam@linaro.org>, <quic_psodagud@quicinc.com>,
        <quic_nkela@quicinc.com>, <gregkh@linuxfoundation.org>,
        <linux-arm-msm@vger.kernel.org>, <quic_wcheng@quicinc.com>,
        <linux-phy@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
        <vkoul@kernel.org>
References: <1709657858-8563-1-git-send-email-quic_sriramd@quicinc.com>
 <1709657858-8563-2-git-send-email-quic_sriramd@quicinc.com>
 <86371fc0-ef49-4dc9-b98c-7c5131cd1227@linaro.org>
 <986b49f5-6a4a-430d-ba6a-2f387f2a262a@quicinc.com>
 <84618a4a-be20-4c59-badc-15fa65debf4c@linaro.org>
 <e441e458-a90b-495f-abf1-448620c14c85@quicinc.com>
From: Sriram Dash <quic_sriramd@quicinc.com>
In-Reply-To: <e441e458-a90b-495f-abf1-448620c14c85@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit

On 3/6/2024 1:03 PM, Krishna Kurapati PSSNV wrote:
> 
> 
> On 3/6/2024 12:33 PM, Krzysztof Kozlowski wrote:
>> On 05/03/2024 19:03, Sriram Dash wrote:
>>> On 3/5/2024 10:37 PM, Krzysztof Kozlowski wrote:
>>>> On 05/03/2024 17:57, Sriram Dash wrote:
>>>>> Some target systems allow multiple resources to be managed by 
>>>>> firmware.
>>>>> On these targets, tasks related to clocks, regulators, resets, and
>>>>> interconnects can be delegated to the firmware, while the remaining
>>>>> responsibilities are handled by Linux.
>>>>>
>>>>> To support the management of partial resources in Linux and leave 
>>>>> the rest
>>>>> to firmware, multiple power domains are introduced. Each power 
>>>>> domain can
>>>>> manage one or more resources, depending on the specific use case.
>>>>>
>>>>> These power domains handle SCMI calls to the firmware, enabling the
>>>>> activation and deactivation of firmware-managed resources.
>>>>>
>>>>> Signed-off-by: Sriram Dash <quic_sriramd@quicinc.com>
>>>>> ---
>>>>>    .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml        | 74 
>>>>> ++++++++++++++++------
>>>>>    .../bindings/phy/qcom,usb-snps-femto-v2.yaml       | 49 
>>>>> ++++++++++++--
>>>>>    .../devicetree/bindings/usb/qcom,dwc3.yaml         | 37 ++++++++++-
>>>>>    3 files changed, 130 insertions(+), 30 deletions(-)
>>>>>
>>>>> diff --git 
>>>>> a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
>>>>> index 1e2d4dd..53b9ba9 100644
>>>>> --- 
>>>>> a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
>>>>> +++ 
>>>>> b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
>>>>> @@ -44,7 +44,32 @@ properties:
>>>>>        maxItems: 5
>>>>>      power-domains:
>>>>> -    maxItems: 1
>>>>> +    description: specifies a phandle to PM domain provider node
>>>>
>>>> Please drop all redundant descriptions. Adding them is not even related
>>>> to this patch.
>>>>
>>>
>>> Thanks Krzysztof for the super quick response !
>>> Sure. will drop the description for power-domain
>>> and power-doamin-names.
>>>
>>>>> +    minItems: 1
>>>>> +    maxItems: 2
>>>>> +
>>>>> +  power-domain-names:
>>>>> +    description:
>>>>> +      A list of power domain name strings sorted in the same order 
>>>>> as the
>>>>> +      power-domains property.
>>>>> +
>>>>> +      For platforms where some resource are firmware managed, the 
>>>>> name
>>>>> +      corresponding to the index of an SCMI domain provider can be
>>>>> +      "usb_core" or "usb_transfer".
>>>>> +    items:
>>>>> +      - const: usb_core
>>>>> +      - const: usb_transfer
>>>>
>>>> How is this related to fw-managed? I fail to see it. Don't mix
>>>> independent problems in one patch.
>>>>
>>>
>>> Some of the the resources like clocks, regulators, etc will be
>>> controlled by the firmware instead of OS. However, some resources
>>> still will be controlled by OS (interrupts for example).
>>>
>>> So, to support the management of partial resources in Linux, and
>>> offload the other resource management to firmware, multiple power
>>> domains are introduced. They will be corresponding to different
>>> hw blocks.
>>>
>>> Do you suggest splitting the addition of power-domain-names and
>>> addition of fw-managed property in separate patches for bindings!
>>>
>>>>> +
>>>>> +  qmp,fw-managed:
>>>>
>>>> Please do not upstream vendor code directly, but perform basic
>>>> adjustment to upstream Linux kernel. There is no such company as gmp.
>>>>
>>>> Run this first through your internal review process.
>>>>
>>>
>>> This property is newly introduced for the qmp superspeed phy and
>>> similar dt properties are introduced for hsphy and dwc3 qcom
>>> controller glue layer driver. It will govern the suspend/ resume
>>> of the resources (by firmware or OS) as required.
>>
>> So that's your answer to "there is no such company as gmp"? It's not
>> relevant at all.
>>
>> Please run it through internal review first.
>>
>>
> Hi Sriram,
> 
>   QMP indicates the driver name "QMP Phy" (for super speed mode of 
> operation), not the company name. If this property is for identification 
> of fw management to be done / (or not to be done) in QMP Phy, then 
> please rename is appropriately. Check how other properties are defined 
> in the QMP bindings and add "qcom" at beginning preferably. For 
> reference as to how properties were added to femto phy driver: [1].
> 
> [1]: 
> https://patchwork.kernel.org/project/linux-arm-msm/patch/1662480933-12326-2-git-send-email-quic_kriskura@quicinc.com/
> 

Thanks for the references Krzysztof/Krishna.
I got suggestion from Dmitry on the use of the
property too. Indeed we will be the scmi based
clock/ PD management on SA8775. So, we will
introduce a new compatible string and take the
decision in driver accordingly.


> Regards,
> Krishna,