[PATCH] USB: initialize or shutdown PHY when add or remove host controller

[Alan Stern]

On Tue, 18 Jun 2013, Felipe Balbi wrote:

> HI,
> 
> On Tue, Jun 18, 2013 at 11:23:59AM +0300, Roger Quadros wrote:
> > On 06/18/2013 11:01 AM, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Tue, Jun 18, 2013 at 03:15:01AM -0400, Chao Xie wrote:
> > >> Some controller need software to initialize PHY before add
> > >> host controller, and shut down PHY after remove host controller.
> > >> Add the generic code for these controllers so they do not need
> > >> do it in its own host controller driver.
> > >>
> > >> Signed-off-by: Chao Xie <chao.xie at marvell.com>
> > >> ---
> > >>  drivers/usb/core/hcd.c |   19 ++++++++++++++++++-
> > >>  1 files changed, 18 insertions(+), 1 deletions(-)
> > >>
> > >> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> > >> index d53547d..b26196b 100644
> > >> --- a/drivers/usb/core/hcd.c
> > >> +++ b/drivers/usb/core/hcd.c
> > >> @@ -43,6 +43,7 @@
> > >>  
> > >>  #include <linux/usb.h>
> > >>  #include <linux/usb/hcd.h>
> > >> +#include <linux/usb/phy.h>
> > >>  
> > >>  #include "usb.h"
> > >>  
> > >> @@ -2531,12 +2532,22 @@ int usb_add_hcd(struct usb_hcd *hcd,
> > >>  	 */
> > >>  	set_bit(HCD_FLAG_RH_RUNNING, &hcd->flags);
> > >>  
> > >> +	/* Initialize the PHY before other hardware operation. */
> > >> +	if (hcd->phy) {
> > > 
> > > this looks wrong for two reasons:
> > > 
> > > a) you're not grabbing the PHY here.
> > > 
> > > 	You can't just assume another entity grabbed your PHY for you.
> > 
> > Isn't that done in the controller drivers e.g. ehci-fsl.c, ohci-omap, etc?
> 
> right, and what I'm saying is that it should all be re-factored into
> ehci-hcd core :-)
> 
> > If the controllers don't want HCD core to manage the PHY they can just set it
> > to some error code.
> 
> they shouldn't have the choice, otherwise it'll be a bit of a PITA to
> maintain the code. ehci core tries to grab the PHY, if it's not there,
> try to continue anyway. Assume it's not needed.

Felipe, are all these issues straightened out to your satisfaction?  I
can't tell from the email thread.

Looking through the EHCI glue source files, there appears to be a 
variety of different ways of getting the PHY.  It's not at all clear 
that they can be moved into the ehci-hcd core (or even better, the USB 
core -- which is what Chao is trying to do).

Given that the glue module has to be responsible for getting the PHY,
it should also be responsible for error checking.  So the code added to
hcd.c doesn't need to apply an IS_ERR check; it can simply assume that
if hcd->phy is NULL then either there is no software-controllable PHY
or else the HCD doesn't want the core to manage it.

Alan Stern