2022-04-22 22:36:28

by Sandeep Maheswaram

[permalink] [raw]
Subject: [PATCH v4 0/3] Skip phy initialization for DWC3 USB Controllers

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/[email protected]/
Dropped the dt bindings PATCH 1/3 in v2
https://patchwork.kernel.org/project/linux-arm-msm/cover/[email protected]/

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


2022-04-25 09:37:06

by Pavan Kondeti

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

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/[email protected]/
> Dropped the dt bindings PATCH 1/3 in v2
> https://patchwork.kernel.org/project/linux-arm-msm/cover/[email protected]/
>
> 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

2022-04-26 10:25:21

by Thinh Nguyen

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

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/[email protected]/__;!!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/[email protected]/__;!!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/[email protected]/#R

2022-04-29 23:25:50

by Thinh Nguyen

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

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/[email protected]/__;!!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/[email protected]/__;!!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

2022-05-03 00:07:11

by Pavan Kondeti

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

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/[email protected]/__;!!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/[email protected]/__;!!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