mbox series

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

Message ID 1650517255-4871-1-git-send-email-quic_c_sanm@quicinc.com
Headers show
Series Skip phy initialization for DWC3 USB Controllers | expand

Message

Sandeep Maheswaram April 21, 2022, 5 a.m. UTC
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(-)

Comments

Pavan Kondeti April 25, 2022, 2:48 a.m. UTC | #1
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
Thinh Nguyen April 26, 2022, 1:12 a.m. UTC | #2
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
Pavan Kondeti April 29, 2022, 3:05 a.m. UTC | #3
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
Thinh Nguyen April 29, 2022, 7:10 p.m. UTC | #4
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