[PATCH v4 0/3] Skip phy initialization for DWC3 USB Controllers

[PATCH v4 0/3] Skip phy initialization for DWC3 USB Controllers

[Sandeep Maheswaram]

Runtime suspend of phy drivers was failing from DWC3 driver as
runtime usage value is 2 because the phy is initialized from
DWC3 core and HCD core.
Some controllers like DWC3 and CDNS3 manage phy in their core drivers.
This property can be set to avoid phy initialization in HCD core.

v4:
Added the device tree binding patch in the series.

v3:
Coming back to this series based on discussion at below thread
https://patchwork.kernel.org/project/linux-arm-msm/patch/1648103831-12347-4-git-send-email-quic_c_sanm@quicinc.com/
Dropped the dt bindings PATCH 1/3 in v2
https://patchwork.kernel.org/project/linux-arm-msm/cover/1636353710-25582-1-git-send-email-quic_c_sanm@quicinc.com/ 

v2:
Updated the commit descriptions.
Changed subject prefix from dwc to dwc3.
Increased props array size.


Sandeep Maheswaram (3):
  dt-bindings: usb: usb-xhci: Add bindings for usb-skip-phy-init
    property
  usb: host: xhci-plat: Add device property to set XHCI_SKIP_PHY_INIT
    quirk
  usb: dwc3: host: Set the property usb-skip-phy-init

 Documentation/devicetree/bindings/usb/usb-xhci.yaml | 4 ++++
 drivers/usb/dwc3/host.c                             | 4 +++-
 drivers/usb/host/xhci-plat.c                        | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v4 1/3] dt-bindings: usb: usb-xhci: Add bindings for usb-skip-phy-init property

[Sandeep Maheswaram]

Adding bindings for usb-skip-phy-init property.
Runtime suspend of phy drivers was failing from DWC3 driver as
runtime usage value is 2 because the phy is initialized from
DWC3 core and HCD core.
Some controllers like DWC3 and CDNS3 manage phy in their core drivers.
This property can be set to avoid phy initialization in HCD core.

Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
---
 Documentation/devicetree/bindings/usb/usb-xhci.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.yaml b/Documentation/devicetree/bindings/usb/usb-xhci.yaml
index 965f87f..a64d29f 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.yaml
@@ -25,6 +25,10 @@ properties:
     description: Set if the controller has broken port disable mechanism
     type: boolean
 
+  usb-skip-phy-init:
+    description: Set if the phy initialization is managed by controller
+    type: boolean
+
   imod-interval-ns:
     description: Interrupt moderation interval
     default: 5000
-- 
2.7.4

Re: [PATCH v4 1/3] dt-bindings: usb: usb-xhci: Add bindings for usb-skip-phy-init property

[Rob Herring]

On Thu, Apr 21, 2022 at 10:30:53AM +0530, Sandeep Maheswaram wrote:
> Adding bindings for usb-skip-phy-init property.
> Runtime suspend of phy drivers was failing from DWC3 driver as
> runtime usage value is 2 because the phy is initialized from
> DWC3 core and HCD core.
> Some controllers like DWC3 and CDNS3 manage phy in their core drivers.
> This property can be set to avoid phy initialization in HCD core.
> 
> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
> ---
>  Documentation/devicetree/bindings/usb/usb-xhci.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.yaml b/Documentation/devicetree/bindings/usb/usb-xhci.yaml
> index 965f87f..a64d29f 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.yaml
> @@ -25,6 +25,10 @@ properties:
>      description: Set if the controller has broken port disable mechanism
>      type: boolean
>  
> +  usb-skip-phy-init:
> +    description: Set if the phy initialization is managed by controller
> +    type: boolean
> +

This should be implied by platform specific compatibles.

Rob

[PATCH v4 2/3] usb: host: xhci-plat: Add device property to set XHCI_SKIP_PHY_INIT quirk

[Sandeep Maheswaram]

Add device property usb-skip-phy-init to check and set XHCI_SKIP_PHY_INIT
quirk.
Runtime suspend of phy drivers was failing from DWC3 driver as
runtime usage value is 2 because the phy is initialized from
DWC3 core and HCD core.
Some controllers like DWC3 and CDNS3 manage phy in their core drivers.
This property can be set to avoid phy initialization in HCD core.

Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
---
 drivers/usb/host/xhci-plat.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 649ffd8..7624ed9 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -305,6 +305,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
 		if (device_property_read_bool(tmpdev, "quirk-broken-port-ped"))
 			xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
+		if (device_property_read_bool(tmpdev, "usb-skip-phy-init"))
+			xhci->quirks |= XHCI_SKIP_PHY_INIT;
+
 		device_property_read_u32(tmpdev, "imod-interval-ns",
 					 &xhci->imod_interval);
 	}
-- 
2.7.4

[PATCH v4 3/3] usb: dwc3: host: Set the property usb-skip-phy-init

[Sandeep Maheswaram]

Runtime suspend of phy drivers was failing from DWC3 driver as
runtime usage value is 2 because the phy is initialized from
DWC3 core and HCD core.
Some controllers like DWC3 and CDNS3 manage phy in their core drivers.
This property can be set to avoid phy initialization in HCD core.

Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
---
 drivers/usb/dwc3/host.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 3902b56..eebf203 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -68,7 +68,7 @@ static int dwc3_host_get_irq(struct dwc3 *dwc)
 
 int dwc3_host_init(struct dwc3 *dwc)
 {
-	struct property_entry	props[4];
+	struct property_entry	props[5];
 	struct platform_device	*xhci;
 	int			ret, irq;
 	int			prop_idx = 0;
@@ -115,6 +115,8 @@ int dwc3_host_init(struct dwc3 *dwc)
 	if (DWC3_VER_IS_WITHIN(DWC3, ANY, 300A))
 		props[prop_idx++] = PROPERTY_ENTRY_BOOL("quirk-broken-port-ped");
 
+	props[prop_idx++] = PROPERTY_ENTRY_BOOL("usb-skip-phy-init");
+
 	if (prop_idx) {
 		ret = device_create_managed_software_node(&xhci->dev, props, NULL);
 		if (ret) {
-- 
2.7.4

Re: [PATCH v4 0/3] Skip phy initialization for DWC3 USB Controllers

[Pavan Kondeti]

Hi Mathias,

On Thu, Apr 21, 2022 at 10:30:52AM +0530, Sandeep Maheswaram wrote:
> Runtime suspend of phy drivers was failing from DWC3 driver as
> runtime usage value is 2 because the phy is initialized from
> DWC3 core and HCD core.
> Some controllers like DWC3 and CDNS3 manage phy in their core drivers.
> This property can be set to avoid phy initialization in HCD core.
> 
> v4:
> Added the device tree binding patch in the series.
> 
> v3:
> Coming back to this series based on discussion at below thread
> https://patchwork.kernel.org/project/linux-arm-msm/patch/1648103831-12347-4-git-send-email-quic_c_sanm@quicinc.com/
> Dropped the dt bindings PATCH 1/3 in v2
> https://patchwork.kernel.org/project/linux-arm-msm/cover/1636353710-25582-1-git-send-email-quic_c_sanm@quicinc.com/ 
> 
> v2:
> Updated the commit descriptions.
> Changed subject prefix from dwc to dwc3.
> Increased props array size.
> 
> 
> Sandeep Maheswaram (3):
>   dt-bindings: usb: usb-xhci: Add bindings for usb-skip-phy-init
>     property
>   usb: host: xhci-plat: Add device property to set XHCI_SKIP_PHY_INIT
>     quirk
>   usb: dwc3: host: Set the property usb-skip-phy-init
> 
>  Documentation/devicetree/bindings/usb/usb-xhci.yaml | 4 ++++
>  drivers/usb/dwc3/host.c                             | 4 +++-
>  drivers/usb/host/xhci-plat.c                        | 3 +++
>  3 files changed, 10 insertions(+), 1 deletion(-)
> 

This is the latest series with bindings added as per Greg's comment. Can you
please pick up this series if you don't have any further comments.

Thanks,
Pavan

Re: [PATCH v4 0/3] Skip phy initialization for DWC3 USB Controllers

[Thinh Nguyen]

Hi,

Pavan Kondeti wrote:
> Hi Mathias,
> 
> On Thu, Apr 21, 2022 at 10:30:52AM +0530, Sandeep Maheswaram wrote:
>> Runtime suspend of phy drivers was failing from DWC3 driver as
>> runtime usage value is 2 because the phy is initialized from
>> DWC3 core and HCD core.
>> Some controllers like DWC3 and CDNS3 manage phy in their core drivers.
>> This property can be set to avoid phy initialization in HCD core.
>>
>> v4:
>> Added the device tree binding patch in the series.
>>
>> v3:
>> Coming back to this series based on discussion at below thread
>> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-arm-msm/patch/1648103831-12347-4-git-send-email-quic_c_sanm@quicinc.com/__;!!A4F2R9G_pg!fykTNTBuKk9ci6zKdcuQNbuZQdVi_HekU3jetzud-PQVhbRaVhhZHKz0k_LfG0cgwaX4bQM5bLI0ep6tYyikgvYK7b5SdA$ 
>> Dropped the dt bindings PATCH 1/3 in v2
>> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-arm-msm/cover/1636353710-25582-1-git-send-email-quic_c_sanm@quicinc.com/__;!!A4F2R9G_pg!fykTNTBuKk9ci6zKdcuQNbuZQdVi_HekU3jetzud-PQVhbRaVhhZHKz0k_LfG0cgwaX4bQM5bLI0ep6tYyikgva2VXahOQ$  
>>
>> v2:
>> Updated the commit descriptions.
>> Changed subject prefix from dwc to dwc3.
>> Increased props array size.
>>
>>
>> Sandeep Maheswaram (3):
>>   dt-bindings: usb: usb-xhci: Add bindings for usb-skip-phy-init
>>     property
>>   usb: host: xhci-plat: Add device property to set XHCI_SKIP_PHY_INIT
>>     quirk
>>   usb: dwc3: host: Set the property usb-skip-phy-init
>>
>>  Documentation/devicetree/bindings/usb/usb-xhci.yaml | 4 ++++
>>  drivers/usb/dwc3/host.c                             | 4 +++-
>>  drivers/usb/host/xhci-plat.c                        | 3 +++
>>  3 files changed, 10 insertions(+), 1 deletion(-)
>>
> 
> This is the latest series with bindings added as per Greg's comment. Can you
> please pick up this series if you don't have any further comments.
> 

We've had this conversation going on for a while. Seems there's no good
one solution with everyone fully getting on-board.

I've tried to get some of the quirks out before also, but ran into the
same problem. [1]

As Mathias noted [2] before, maybe we can create a new xhci-snps
platform glue driver.

The problem with the current implementation is passing dwc3's related
info to xhci-plat generic driver is very clunky. We can teach the new
glue driver with all the info necessary to drive the controller.

We can just pass the controller's version (and subversion) as a property
for platform device. This way, we can:

1) Separate the quirks from xhci-plat glue. Most common quirks can be
detected just base on the controller's version

2) Avoid having to create duplicate "snps,*" properties

3) Get access to the common xhci quirk flags while maintain abstraction

4) Potentially add compatibility string as part of the controller's
version and let the glue driver handle the rest

5) Reduce introducing new "quirks" in the future

I can get started with this. Let me know if you have any comment.

Thanks,
Thinh

[1] https://lore.kernel.org/linux-usb/0fb179b977cd187f003ae18adf01bccf09d74092.1618014279.git.Thinh.Nguyen@synopsys.com/T/#ma5f7bdf29cf84b5a0077a4a0857ceb5dfe0c8564
[2] https://lore.kernel.org/linux-usb/76ecefd7-d294-485a-1e2b-e5e556e2a3f7@linux.intel.com/#R

Re: [PATCH v4 0/3] Skip phy initialization for DWC3 USB Controllers

[Pavan Kondeti]

Hi Thinh,

On Tue, Apr 26, 2022 at 01:12:17AM +0000, Thinh Nguyen wrote:
> Hi,
> 
> Pavan Kondeti wrote:
> > Hi Mathias,
> > 
> > On Thu, Apr 21, 2022 at 10:30:52AM +0530, Sandeep Maheswaram wrote:
> >> Runtime suspend of phy drivers was failing from DWC3 driver as
> >> runtime usage value is 2 because the phy is initialized from
> >> DWC3 core and HCD core.
> >> Some controllers like DWC3 and CDNS3 manage phy in their core drivers.
> >> This property can be set to avoid phy initialization in HCD core.
> >>
> >> v4:
> >> Added the device tree binding patch in the series.
> >>
> >> v3:
> >> Coming back to this series based on discussion at below thread
> >> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-arm-msm/patch/1648103831-12347-4-git-send-email-quic_c_sanm@quicinc.com/__;!!A4F2R9G_pg!fykTNTBuKk9ci6zKdcuQNbuZQdVi_HekU3jetzud-PQVhbRaVhhZHKz0k_LfG0cgwaX4bQM5bLI0ep6tYyikgvYK7b5SdA$ 
> >> Dropped the dt bindings PATCH 1/3 in v2
> >> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-arm-msm/cover/1636353710-25582-1-git-send-email-quic_c_sanm@quicinc.com/__;!!A4F2R9G_pg!fykTNTBuKk9ci6zKdcuQNbuZQdVi_HekU3jetzud-PQVhbRaVhhZHKz0k_LfG0cgwaX4bQM5bLI0ep6tYyikgva2VXahOQ$  
> >>
> >> v2:
> >> Updated the commit descriptions.
> >> Changed subject prefix from dwc to dwc3.
> >> Increased props array size.
> >>
> >>
> >> Sandeep Maheswaram (3):
> >>   dt-bindings: usb: usb-xhci: Add bindings for usb-skip-phy-init
> >>     property
> >>   usb: host: xhci-plat: Add device property to set XHCI_SKIP_PHY_INIT
> >>     quirk
> >>   usb: dwc3: host: Set the property usb-skip-phy-init
> >>
> >>  Documentation/devicetree/bindings/usb/usb-xhci.yaml | 4 ++++
> >>  drivers/usb/dwc3/host.c                             | 4 +++-
> >>  drivers/usb/host/xhci-plat.c                        | 3 +++
> >>  3 files changed, 10 insertions(+), 1 deletion(-)
> >>
> > 
> > This is the latest series with bindings added as per Greg's comment. Can you
> > please pick up this series if you don't have any further comments.
> > 
> 
> We've had this conversation going on for a while. Seems there's no good
> one solution with everyone fully getting on-board.
> 
> I've tried to get some of the quirks out before also, but ran into the
> same problem. [1]
> 
> As Mathias noted [2] before, maybe we can create a new xhci-snps
> platform glue driver.
> 
> The problem with the current implementation is passing dwc3's related
> info to xhci-plat generic driver is very clunky. We can teach the new
> glue driver with all the info necessary to drive the controller.
> 
> We can just pass the controller's version (and subversion) as a property
> for platform device. This way, we can:
> 
> 1) Separate the quirks from xhci-plat glue. Most common quirks can be
> detected just base on the controller's version
> 
> 2) Avoid having to create duplicate "snps,*" properties
> 
> 3) Get access to the common xhci quirk flags while maintain abstraction
> 
> 4) Potentially add compatibility string as part of the controller's
> version and let the glue driver handle the rest
> 
> 5) Reduce introducing new "quirks" in the future
> 
> I can get started with this. Let me know if you have any comment.

Sorry, could not reply earlier. The proposal sounds good to me.

The xhci-plat is a thin wrapper, so having a separate wrapper for SNPS
controller is definitely not an overkill and gives lot of flexibility
in abstracting dwc3 specifics. Also dwc3/host.c becomes just a platform
device creation wrapper and xHC specifics are completely taken out.

Thanks,
Pavan

Re: [PATCH v4 0/3] Skip phy initialization for DWC3 USB Controllers

[Thinh Nguyen]

Pavan Kondeti wrote:
> Hi Thinh,
> 
> On Tue, Apr 26, 2022 at 01:12:17AM +0000, Thinh Nguyen wrote:
>> Hi,
>>
>> Pavan Kondeti wrote:
>>> Hi Mathias,
>>>
>>> On Thu, Apr 21, 2022 at 10:30:52AM +0530, Sandeep Maheswaram wrote:
>>>> Runtime suspend of phy drivers was failing from DWC3 driver as
>>>> runtime usage value is 2 because the phy is initialized from
>>>> DWC3 core and HCD core.
>>>> Some controllers like DWC3 and CDNS3 manage phy in their core drivers.
>>>> This property can be set to avoid phy initialization in HCD core.
>>>>
>>>> v4:
>>>> Added the device tree binding patch in the series.
>>>>
>>>> v3:
>>>> Coming back to this series based on discussion at below thread
>>>> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-arm-msm/patch/1648103831-12347-4-git-send-email-quic_c_sanm@quicinc.com/__;!!A4F2R9G_pg!fykTNTBuKk9ci6zKdcuQNbuZQdVi_HekU3jetzud-PQVhbRaVhhZHKz0k_LfG0cgwaX4bQM5bLI0ep6tYyikgvYK7b5SdA$ 
>>>> Dropped the dt bindings PATCH 1/3 in v2
>>>> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-arm-msm/cover/1636353710-25582-1-git-send-email-quic_c_sanm@quicinc.com/__;!!A4F2R9G_pg!fykTNTBuKk9ci6zKdcuQNbuZQdVi_HekU3jetzud-PQVhbRaVhhZHKz0k_LfG0cgwaX4bQM5bLI0ep6tYyikgva2VXahOQ$  
>>>>
>>>> v2:
>>>> Updated the commit descriptions.
>>>> Changed subject prefix from dwc to dwc3.
>>>> Increased props array size.
>>>>
>>>>
>>>> Sandeep Maheswaram (3):
>>>>   dt-bindings: usb: usb-xhci: Add bindings for usb-skip-phy-init
>>>>     property
>>>>   usb: host: xhci-plat: Add device property to set XHCI_SKIP_PHY_INIT
>>>>     quirk
>>>>   usb: dwc3: host: Set the property usb-skip-phy-init
>>>>
>>>>  Documentation/devicetree/bindings/usb/usb-xhci.yaml | 4 ++++
>>>>  drivers/usb/dwc3/host.c                             | 4 +++-
>>>>  drivers/usb/host/xhci-plat.c                        | 3 +++
>>>>  3 files changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>
>>> This is the latest series with bindings added as per Greg's comment. Can you
>>> please pick up this series if you don't have any further comments.
>>>
>>
>> We've had this conversation going on for a while. Seems there's no good
>> one solution with everyone fully getting on-board.
>>
>> I've tried to get some of the quirks out before also, but ran into the
>> same problem. [1]
>>
>> As Mathias noted [2] before, maybe we can create a new xhci-snps
>> platform glue driver.
>>
>> The problem with the current implementation is passing dwc3's related
>> info to xhci-plat generic driver is very clunky. We can teach the new
>> glue driver with all the info necessary to drive the controller.
>>
>> We can just pass the controller's version (and subversion) as a property
>> for platform device. This way, we can:
>>
>> 1) Separate the quirks from xhci-plat glue. Most common quirks can be
>> detected just base on the controller's version
>>
>> 2) Avoid having to create duplicate "snps,*" properties
>>
>> 3) Get access to the common xhci quirk flags while maintain abstraction
>>
>> 4) Potentially add compatibility string as part of the controller's
>> version and let the glue driver handle the rest
>>
>> 5) Reduce introducing new "quirks" in the future
>>
>> I can get started with this. Let me know if you have any comment.
> 
> Sorry, could not reply earlier. The proposal sounds good to me.
> 
> The xhci-plat is a thin wrapper, so having a separate wrapper for SNPS
> controller is definitely not an overkill and gives lot of flexibility
> in abstracting dwc3 specifics. Also dwc3/host.c becomes just a platform
> device creation wrapper and xHC specifics are completely taken out.
> 

Sure. I'll be away for a few weeks and come back by the end of May. I
can do that then.

Thanks,
Thinh