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@xxxxxxxxxxx> > > >> --- > > >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html