[PATCH 0/3] USB DWC3 QCOM Multi power domain support

[PATCH 0/3] USB DWC3 QCOM Multi power domain support

[Sandeep Maheswaram]

Add multi pd support to set performance state for cx domain
to maintain minimum corner voltage for USB clocks.

Add corresponding dt bindings, driver changes and dt changes.

Sandeep Maheswaram (3):
  dt-bindings: usb: qcom,dwc3: Add multi-pd bindings for dwc3 qcom
  usb: dwc3: qcom: Add multi-pd support
  arm64: dts: qcom: sc7280: Add cx power domain support

 .../devicetree/bindings/usb/qcom,dwc3.yaml         | 13 +++++-
 arch/arm64/boot/dts/qcom/sc7280.dtsi               |  5 ++-
 drivers/usb/dwc3/dwc3-qcom.c                       | 49 ++++++++++++++++++++++
 3 files changed, 65 insertions(+), 2 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 1/3] dt-bindings: usb: qcom,dwc3: Add multi-pd bindings for dwc3 qcom

[Sandeep Maheswaram]

Add multi pd bindings to set performance state for cx domain
to maintain minimum corner voltage for USB clocks.

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index e70afc4..838d9c4 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -41,7 +41,18 @@ properties:
 
   power-domains:
     description: specifies a phandle to PM domain provider node
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: optional,cx power domain
+      - description: USB gdsc power domain
+
+  power-domain-names:
+     items:
+      - const: cx
+      - const: usb_gdsc
+
+  required-opps:
+    description: specifies the performance state to cx power domain
 
   clocks:
     description:
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/3] dt-bindings: usb: qcom,dwc3: Add multi-pd bindings for dwc3 qcom

[Doug Anderson]

Hi,

On Mon, Aug 30, 2021 at 10:55 AM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>
> Add multi pd bindings to set performance state for cx domain
> to maintain minimum corner voltage for USB clocks.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> index e70afc4..838d9c4 100644
> --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> @@ -41,7 +41,18 @@ properties:
>
>    power-domains:
>      description: specifies a phandle to PM domain provider node
> -    maxItems: 1
> +    minItems: 1
> +    items:
> +      - description: optional,cx power domain
> +      - description: USB gdsc power domain

You need to re-order the above. The optional one needs to be second, not first.


> +  power-domain-names:
> +     items:
> +      - const: cx
> +      - const: usb_gdsc

Why do you need the names at all? The ordering of power-domains is
well defined and there are no holes in it and there are no legacy
reasons for having the names (like there are for clocks), so you
should drop. This is much like reg-names and I always point people to
this message from Rob Herring about reg-names:

https://lore.kernel.org/r/CAL_Jsq+MMunmVWqeW9v2RyzsMKP+=kMzeTHNMG4JDHM7Fy0HBg@mail.gmail.com/

You'll have to change your driver to use dev_pm_domain_attach_by_id()
but that should be fine.

-Doug

Re: [PATCH 1/3] dt-bindings: usb: qcom,dwc3: Add multi-pd bindings for dwc3 qcom

[Sandeep Maheswaram]

On 8/31/2021 1:37 AM, Doug Anderson wrote:
> Hi,
>
> On Mon, Aug 30, 2021 at 10:55 AM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>> Add multi pd bindings to set performance state for cx domain
>> to maintain minimum corner voltage for USB clocks.
>>
>> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
>> ---
>>   Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>> index e70afc4..838d9c4 100644
>> --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>> @@ -41,7 +41,18 @@ properties:
>>
>>     power-domains:
>>       description: specifies a phandle to PM domain provider node
>> -    maxItems: 1
>> +    minItems: 1
>> +    items:
>> +      - description: optional,cx power domain
>> +      - description: USB gdsc power domain
> You need to re-order the above. The optional one needs to be second, not first.
>
I wanted to use required-opps for cx domain only. so I have put it first 
in order.
>> +  power-domain-names:
>> +     items:
>> +      - const: cx
>> +      - const: usb_gdsc
> Why do you need the names at all? The ordering of power-domains is
> well defined and there are no holes in it and there are no legacy
> reasons for having the names (like there are for clocks), so you
> should drop. This is much like reg-names and I always point people to
> this message from Rob Herring about reg-names:
>
> https://lore.kernel.org/r/CAL_Jsq+MMunmVWqeW9v2RyzsMKP+=kMzeTHNMG4JDHM7Fy0HBg@mail.gmail.com/
>
> You'll have to change your driver to use dev_pm_domain_attach_by_id()
> but that should be fine.
>
> -Doug

Ok..I will try using  dev_pm_domain_attach_by_id()

Re: [PATCH 1/3] dt-bindings: usb: qcom,dwc3: Add multi-pd bindings for dwc3 qcom

[Rajendra Nayak]

On 9/6/2021 2:45 PM, Sandeep Maheswaram wrote:
> 
> On 8/31/2021 1:37 AM, Doug Anderson wrote:
>> Hi,
>>
>> On Mon, Aug 30, 2021 at 10:55 AM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>>> Add multi pd bindings to set performance state for cx domain
>>> to maintain minimum corner voltage for USB clocks.
>>>
>>> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
>>> ---
>>>   Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 13 ++++++++++++-
>>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>>> index e70afc4..838d9c4 100644
>>> --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>>> @@ -41,7 +41,18 @@ properties:
>>>
>>>     power-domains:
>>>       description: specifies a phandle to PM domain provider node
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    items:
>>> +      - description: optional,cx power domain
>>> +      - description: USB gdsc power domain
>> You need to re-order the above. The optional one needs to be second, not first.
>>
> I wanted to use required-opps for cx domain only. so I have put it first in order.

You can always put a <> for the power-domains for which there are no required-opps

+			power-domain-names = "usb_gdsc", "cx";
+
+			required-opps = <>, <&rpmhpd_opp_nom>;

>>> +  power-domain-names:
>>> +     items:
>>> +      - const: cx
>>> +      - const: usb_gdsc
>> Why do you need the names at all? The ordering of power-domains is
>> well defined and there are no holes in it and there are no legacy
>> reasons for having the names (like there are for clocks), so you
>> should drop. This is much like reg-names and I always point people to
>> this message from Rob Herring about reg-names:
>>
>> https://lore.kernel.org/r/CAL_Jsq+MMunmVWqeW9v2RyzsMKP+=kMzeTHNMG4JDHM7Fy0HBg@mail.gmail.com/
>>
>> You'll have to change your driver to use dev_pm_domain_attach_by_id()
>> but that should be fine.
>>
>> -Doug
> 
> Ok..I will try using  dev_pm_domain_attach_by_id()
> 
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/3] dt-bindings: usb: qcom,dwc3: Add multi-pd bindings for dwc3 qcom

[Bjorn Andersson]

On Mon 30 Aug 10:54 PDT 2021, Sandeep Maheswaram wrote:

> Add multi pd bindings to set performance state for cx domain
> to maintain minimum corner voltage for USB clocks.
> 
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> index e70afc4..838d9c4 100644
> --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> @@ -41,7 +41,18 @@ properties:
>  
>    power-domains:
>      description: specifies a phandle to PM domain provider node
> -    maxItems: 1
> +    minItems: 1
> +    items:
> +      - description: optional,cx power domain
> +      - description: USB gdsc power domain
> +
> +  power-domain-names:
> +     items:
> +      - const: cx
> +      - const: usb_gdsc

But "usb_gdsc" is a subdomain of "cx", why can't we describe this fact
in gcc?

Regards,
Bjorn

> +
> +  required-opps:
> +    description: specifies the performance state to cx power domain
>  
>    clocks:
>      description:
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 

Re: [PATCH 1/3] dt-bindings: usb: qcom,dwc3: Add multi-pd bindings for dwc3 qcom

[Bjorn Andersson]

On Thu 30 Sep 02:41 PDT 2021, Sandeep Maheswaram wrote:

> 
> On 9/7/2021 7:20 PM, Bjorn Andersson wrote:
> > On Mon 30 Aug 10:54 PDT 2021, Sandeep Maheswaram wrote:
> > 
> > > Add multi pd bindings to set performance state for cx domain
> > > to maintain minimum corner voltage for USB clocks.
> > > 
> > > Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> > > ---
> > >   Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 13 ++++++++++++-
> > >   1 file changed, 12 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> > > index e70afc4..838d9c4 100644
> > > --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
> > > @@ -41,7 +41,18 @@ properties:
> > >     power-domains:
> > >       description: specifies a phandle to PM domain provider node
> > > -    maxItems: 1
> > > +    minItems: 1
> > > +    items:
> > > +      - description: optional,cx power domain
> > > +      - description: USB gdsc power domain
> > > +
> > > +  power-domain-names:
> > > +     items:
> > > +      - const: cx
> > > +      - const: usb_gdsc
> > But "usb_gdsc" is a subdomain of "cx", why can't we describe this fact
> > in gcc?
> > 
> > Regards,
> > Bjorn
> Thanks for your review.
> Any idea on how can this be described in gcc ? Can you point any reference
> for this .
> 

There's a series from Dmitry that defines such a relationship between
MDSS_GDSC and the MMCX domain on SM8250. This seems like a continuation
of that support, given that we have multiple parent domains (cx, mx
etc).

You can find that discussion here:

https://lore.kernel.org/all/20210727202004.712665-1-dmitry.baryshkov@linaro.org/

Regards,
Bjorn

> Regards
> Sandeep
> > > +
> > > +  required-opps:
> > > +    description: specifies the performance state to cx power domain
> > >     clocks:
> > >       description:
> > > -- 
> > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> > > of Code Aurora Forum, hosted by The Linux Foundation
> > > 

[PATCH 2/3] usb: dwc3: qcom: Add multi-pd support

[Sandeep Maheswaram]

Add multi pd support to set performance state for cx domain
to maintain minimum corner voltage for USB clocks.

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
 drivers/usb/dwc3/dwc3-qcom.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 9abbd01..777a647 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -17,6 +17,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
+#include <linux/pm_domain.h>
 #include <linux/usb/of.h>
 #include <linux/reset.h>
 #include <linux/iopoll.h>
@@ -89,6 +90,10 @@ struct dwc3_qcom {
 	bool			pm_suspended;
 	struct icc_path		*icc_path_ddr;
 	struct icc_path		*icc_path_apps;
+	/* power domain for cx */
+	struct device		*pd_cx;
+	/* power domain for usb gdsc */
+	struct device		*pd_usb_gdsc;
 };
 
 static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
@@ -521,6 +526,46 @@ static int dwc3_qcom_setup_irq(struct platform_device *pdev)
 	return 0;
 }
 
+static int dwc3_qcom_attach_pd(struct device *dev)
+{
+	struct dwc3_qcom *qcom = dev_get_drvdata(dev);
+	struct device_link *link;
+
+	/* Do nothing when in a single power domain */
+	if (dev->pm_domain)
+		return 0;
+
+	qcom->pd_cx = dev_pm_domain_attach_by_name(dev, "cx");
+	if (IS_ERR(qcom->pd_cx))
+		return PTR_ERR(qcom->pd_cx);
+	/* Do nothing when power domain missing */
+	if (!qcom->pd_cx)
+		return 0;
+	link = device_link_add(dev, qcom->pd_cx,
+			DL_FLAG_STATELESS |
+			DL_FLAG_PM_RUNTIME |
+			DL_FLAG_RPM_ACTIVE);
+	if (!link) {
+		dev_err(dev, "Failed to add device_link to cx pd.\n");
+		return -EINVAL;
+	}
+
+	qcom->pd_usb_gdsc = dev_pm_domain_attach_by_name(dev, "usb_gdsc");
+	if (IS_ERR(qcom->pd_usb_gdsc))
+		return PTR_ERR(qcom->pd_usb_gdsc);
+
+	link = device_link_add(dev, qcom->pd_usb_gdsc,
+			DL_FLAG_STATELESS |
+			DL_FLAG_PM_RUNTIME |
+			DL_FLAG_RPM_ACTIVE);
+	if (!link) {
+		dev_err(dev, "Failed to add device_link to usb gdsc pd.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
 {
 	struct device		*dev = qcom->dev;
@@ -837,6 +882,10 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (ret)
 		goto interconnect_exit;
 
+	ret = dwc3_qcom_attach_pd(dev);
+	if (ret)
+		goto interconnect_exit;
+
 	device_init_wakeup(&pdev->dev, 1);
 	qcom->is_suspended = false;
 	pm_runtime_set_active(dev);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 2/3] usb: dwc3: qcom: Add multi-pd support

[Felipe Balbi]

Hi,

Sandeep Maheswaram <sanm@codeaurora.org> writes:
> Add multi pd support to set performance state for cx domain
> to maintain minimum corner voltage for USB clocks.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 9abbd01..777a647 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -17,6 +17,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/phy/phy.h>
> +#include <linux/pm_domain.h>
>  #include <linux/usb/of.h>
>  #include <linux/reset.h>
>  #include <linux/iopoll.h>
> @@ -89,6 +90,10 @@ struct dwc3_qcom {
>  	bool			pm_suspended;
>  	struct icc_path		*icc_path_ddr;
>  	struct icc_path		*icc_path_apps;
> +	/* power domain for cx */
> +	struct device		*pd_cx;
> +	/* power domain for usb gdsc */
> +	struct device		*pd_usb_gdsc;
>  };
>  
>  static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
> @@ -521,6 +526,46 @@ static int dwc3_qcom_setup_irq(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static int dwc3_qcom_attach_pd(struct device *dev)
> +{
> +	struct dwc3_qcom *qcom = dev_get_drvdata(dev);
> +	struct device_link *link;
> +
> +	/* Do nothing when in a single power domain */
> +	if (dev->pm_domain)
> +		return 0;
> +
> +	qcom->pd_cx = dev_pm_domain_attach_by_name(dev, "cx");
> +	if (IS_ERR(qcom->pd_cx))
> +		return PTR_ERR(qcom->pd_cx);
> +	/* Do nothing when power domain missing */
> +	if (!qcom->pd_cx)
> +		return 0;
> +	link = device_link_add(dev, qcom->pd_cx,
> +			DL_FLAG_STATELESS |
> +			DL_FLAG_PM_RUNTIME |
> +			DL_FLAG_RPM_ACTIVE);
> +	if (!link) {
> +		dev_err(dev, "Failed to add device_link to cx pd.\n");

do you need to call dev_pm_domain_dettach() here?

> +		return -EINVAL;
> +	}
> +
> +	qcom->pd_usb_gdsc = dev_pm_domain_attach_by_name(dev, "usb_gdsc");
> +	if (IS_ERR(qcom->pd_usb_gdsc))

do you need to call dev_pm_domain_dettach() here?

> +		return PTR_ERR(qcom->pd_usb_gdsc);
> +
> +	link = device_link_add(dev, qcom->pd_usb_gdsc,
> +			DL_FLAG_STATELESS |
> +			DL_FLAG_PM_RUNTIME |
> +			DL_FLAG_RPM_ACTIVE);
> +	if (!link) {
> +		dev_err(dev, "Failed to add device_link to usb gdsc pd.\n");

do you need to call dev_pm_domain_dettach() here?

-- 
balbi

[PATCH 3/3] arm64: dts: qcom: sc7280: Add cx power domain support

[Sandeep Maheswaram]

Add multi pd support to set performance state for cx domain
to maintain minimum corner voltage for USB clocks.

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 53a21d0..7ccccb7 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1374,7 +1374,10 @@
 			interrupt-names = "hs_phy_irq", "dp_hs_phy_irq",
 					  "dm_hs_phy_irq", "ss_phy_irq";
 
-			power-domains = <&gcc GCC_USB30_PRIM_GDSC>;
+			power-domains = <&rpmhpd SC7280_CX>, <&gcc GCC_USB30_PRIM_GDSC>;
+			power-domain-names = "cx", "usb_gdsc";
+
+			required-opps = <&rpmhpd_opp_nom>;
 
 			resets = <&gcc GCC_USB30_PRIM_BCR>;
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation