From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932135AbcGAKQe (ORCPT ); Fri, 1 Jul 2016 06:16:34 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:43171 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752442AbcGAKQX (ORCPT ); Fri, 1 Jul 2016 06:16:23 -0400 Subject: Re: [PATCH] usb: host: Allow EHCI_OMAP to be built-in when USB_GADGET is 'm' To: Felipe Balbi , References: <1467276056-19357-1-git-send-email-rogerq@ti.com> <8760srgf00.fsf@linux.intel.com> <577525BF.3060509@ti.com> <878txloknc.fsf@linux.intel.com> <57762591.8010703@ti.com> <8760spojty.fsf@linux.intel.com> <57762999.8040906@ti.com> <8737ntofu0.fsf@linux.intel.com> CC: , , , , , From: Roger Quadros Message-ID: <577642AE.7010907@ti.com> Date: Fri, 1 Jul 2016 13:15:10 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <8737ntofu0.fsf@linux.intel.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/07/16 12:41, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >>>>>>>> NOP_USB_XCEIV is used not only by gadget drivers but by >>>>>>>> host drivers as well e.g. EHCI_OMAP. >>>>>>>> >>>>>>>> commit 5a8d651a2bde ("usb: gadget: move gadget API functions to udc-core") >>>>>>>> made it so that NOP_USB_XCEIV can't be built-in if USB_GADGET is 'm'. >>>>>>>> But this prevents EHCI_OMAP to be built-in if USB_GADGET is 'm'. >>>>>>>> >>>>>>>> Fix this undesired behaviour by moving usb_gadget_vbus_connect/disconnect() >>>>>>>> to usb/gadget.h so that NOP_USB_XCEIV has no build dependency >>>>>>>> on USB_GADGET. >>>>>>>> >>>>>>>> Retain the original Kconfig behaviour i.e. NOP_USB_XCEIV is selected >>>>>>>> by drivers that need it. >>>>>>> >>>>>>> no, this is the wrong way to fix this. NOP _has_ a dependency on the >>>>>>> Gadget API if it calls Gadget API functions. Dependencies are proper. >>>>>>> >>>>>>> Maybe the reason for the problem is that we ended up adding far too much >>>>>>> code to phy-generic.c itself. Maybe it shouldn't know about clks and >>>>>>> interrupts. The original idea of that driver was to simply satisfy a >>>>>>> requirement to have a valid transceiver by some platforms. Maybe we >>>>>>> should fix that instead. Moving functions around to workaround a problem >>>>>>> is not the way to go, sorry. >>>>>>> >>>>>> OK but something that was working all these years is broken by your >>>>>> patch. >>>>>> Do you mind fixing it please? Or at least let me know how you want to >>>>>> get it fixed. >>>>> >>>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig >>>>> index 2e710a4cca52..89fd095ca33d 100644 >>>>> --- a/drivers/usb/host/Kconfig >>>>> +++ b/drivers/usb/host/Kconfig >>>>> @@ -180,7 +180,7 @@ config USB_EHCI_MXC >>>>> config USB_EHCI_HCD_OMAP >>>>> tristate "EHCI support for OMAP3 and later chips" >>>>> depends on ARCH_OMAP >>>>> - depends on NOP_USB_XCEIV >>>>> + depends on USB_PHY >>>>> default y >>>>> ---help--- >>>>> Enables support for the on-chip EHCI controller on >>>>> >>>>> >>>> This doesn't fix the real problem. i.e. we can't have NFS root on pandaboard >>>> and omap5_uevm because network device is on USB EHCI, unless we have >>>> both USB_GADGET and NOP_USB_XCEVI built-in, which is not really that >>>> desirable. >>> >>> Well, NOP calls a symbol that belongs to the Gadget API. So NOP cannot >>> be built-in if gadget API is a module. Satisfy the dependencies and >>> everything will work for you. >>> >>> Don't get me wrong, I see what you mean; but you _are_ relying in a >>> driver that calls into the Gadget API. Consider if it was the other way >>> around: say NOP was calling something from usbcore for whatever reason >>> and MUSB depended on NOP. You wouldn't have MUSB built-in unless NOP and >>> usbcore were also built-in, right? >>> >> I agree with you. As far as EHCI_OMAP is concerned we're only interested >> in configuring the PHY clocks and issuing a GPIO reset. >> As of now because of dependency issues with GADGET, NOP_USB_XCEIV doesn't >> seem to be the right place. I'm OK with using some other driver >> that doesn't have such dependency, even if it breaks DT compatibility with >> some OMAP boards. Or I'll just let EHCI_OMAP do it all by itself. >> >> Unless Tony has any objections. > > I don't have memory of why EHCI-omap is using PHY, actually. Is it > really resetting a PHY? > on omap3-beagle, omap4-panda and omap5-uevm, we have an external USB PHY for the EHCI host. That PHY has it's reset line tied to a GPIO and CLK line tied to one of the SoC clock pins. cheers, -roger