Hi Sandeep, On Thu, Feb 10, 2022 at 11:18:19AM +0530, Sandeep Maheswaram wrote: > Hi Pavan, > > On 2/9/2022 11:31 AM, Sandeep Maheswaram wrote: > > > >On 2/9/2022 11:23 AM, Pavan Kondeti wrote: > >>On Tue, Feb 08, 2022 at 11:11:53AM +0100, Greg Kroah-Hartman wrote: > >>>On Tue, Feb 08, 2022 at 03:34:22PM +0530, Sandeep Maheswaram wrote: > >>>>Hi Greg, > >>>> > >>>>On 1/27/2022 10:28 AM, Sandeep Maheswaram wrote: > >>>>>On 1/26/2022 5:55 PM, Greg Kroah-Hartman wrote: > >>>>>>On Fri, Jan 07, 2022 at 10:27:59AM +0530, Sandeep Maheswaram wrote: > >>>>>>>On 1/6/2022 7:55 PM, Greg Kroah-Hartman wrote: > >>>>>>>>On Wed, Dec 22, 2021 at 11:39:43AM +0530, Sandeep Maheswaram > >>>>>>>>wrote: > >>>>>>>>>Set XHCI_SKIP_PHY_INIT quirk to avoid phy initialization twice. > >>>>>>>>>Runtime suspend of phy drivers was failing from DWC3 > >>>>>>>>>driver as runtime > >>>>>>>>>usage value is 2 because the phy is initialized from > >>>>>>>>>DWC3 and HCD core. > >>>>>>>>>DWC3 manages phy in their core drivers. Set this quirk to > >>>>>>>>>avoid phy > >>>>>>>>>initialization in HCD core. > >>>>>>>>> > >>>>>>>>>Signed-off-by: Sandeep Maheswaram <quic_c_sanm@xxxxxxxxxxx> > >>>>>>>>>--- > >>>>>>>>>v5: > >>>>>>>>>Added comment to explain the change done. > >>>>>>>>>v4: > >>>>>>>>>Changed pdev->dev.parent->of_node to sysdev->of_node > >>>>>>>>> > >>>>>>>>> drivers/usb/host/xhci-plat.c | 8 ++++++++ > >>>>>>>>> 1 file changed, 8 insertions(+) > >>>>>>>>> > >>>>>>>>>diff --git a/drivers/usb/host/xhci-plat.c > >>>>>>>>>b/drivers/usb/host/xhci-plat.c > >>>>>>>>>index c1edcc9..e6014d4 100644 > >>>>>>>>>--- a/drivers/usb/host/xhci-plat.c > >>>>>>>>>+++ b/drivers/usb/host/xhci-plat.c > >>>>>>>>>@@ -327,6 +327,14 @@ static int xhci_plat_probe(struct > >>>>>>>>>platform_device *pdev) > >>>>>>>>>&xhci->imod_interval); > >>>>>>>>> } > >>>>>>>>>+ /* > >>>>>>>>>+ * Set XHCI_SKIP_PHY_INIT quirk to avoid phy > >>>>>>>>>initialization twice. > >>>>>>>>>+ * DWC3 manages phy in their core drivers. Set this > >>>>>>>>>quirk to avoid phy > >>>>>>>>>+ * initialization in HCD core. > >>>>>>>>>+ */ > >>>>>>>>>+ if (of_device_is_compatible(sysdev->of_node, "snps,dwc3")) > >>>>>>>>>+ xhci->quirks |= XHCI_SKIP_PHY_INIT; > >>>>>>>>>+ > >>>>>>>>Why is this function caring about dwc3 stuff? Shoudn't this be a > >>>>>>>>"generic" device property instead of this device-specific one? > >>>>>>>> > >>>>>>>>thanks, > >>>>>>>> > >>>>>>>>greg k-h > >>>>>>>This quirk is set only if required for some controllers (eg: > >>>>>>>dwc3 & cdns3). > >>>>>>> > >>>>>>>Please check below commit. > >>>>>>> > >>>>>>>https://lore.kernel.org/all/20200918131752.16488-5-mathias.nyman@xxxxxxxxxxxxxxx/ > >>>>>>> > >>>>>>> > >>>>>>That commit has nothing to do with a specific "dwc3" quirk anywhere. > >>>>>>Why not set this flag in the specific platform xhci driver > >>>>>>instead where > >>>>>>it belongs? > >>>>>> > >>>>>>thanks, > >>>>>> > >>>>>>greg k-h > >>>>>There is no specific xhci platform driver for dwc3 controllers. > >>>>> > >>>>>dwc3 controllers use xhci-plat driver . > >>>>> > >>>>>We can add this quirk in usb/dwc3/host.c as cdns3 does but that > >>>>>requires > >>>>>tying dwc3 and xhci driver . > >>>>> > >>>>>https://patchwork.kernel.org/project/linux-arm-msm/patch/1633946518-13906-1-git-send-email-sanm@xxxxxxxxxxxxxx/ > >>>>> > >>>>> > >>>>> > >>>>>Regards > >>>>> > >>>>>Sandeep > >>>>> > >>>>> > >>>>Can you suggest any other method to set this quirk for dwc3 > >>>>controllers. > >>>No idea, sorry. > >>Sandeep, > >> > >>I agree with Greg's comments here. The compatible based check to detect > >>dwc3 > >>controller is not elegant. Your proposal of adding a device tree param > >>is > >>overkill, I believe. > >> > >>Greg already gave us a pointer here [1] which I feel is the best > >>approach going > >>forward. We know that xhci-plat is being used by drivers like dwc3, > >>cdns3 and > >>these drivers need to expose their xhci quirks. As Greg suggested, why > >>can't > >>we move xhci quirks definition to include/linux/usb/xhci-quriks.h and > >>directly > >>access from the glue drivers? The attached is the patch (completely > >>untested) > >>for your reference. It will prepare the setup for you to add the private > >>data > >>and quirks in the dwc3 host glue driver. > >> > >>Thanks, > >>Pavan > >> > >>[1] > >>https://patchwork.kernel.org/project/linux-arm-msm/patch/1633946518-13906-1-git-send-email-sanm@xxxxxxxxxxxxxx/ > >> > >> > >Thanks Pavan..will test the patch. > > Tested your patch. It is working fine along with the attached changes. > Your patch looks good to me. Feel free to send the two patches together. Thanks, Pavan