On Fri, Mar 08, 2013 at 05:56:11PM +0200, Felipe Balbi wrote:
> Hi,
>
> On Fri, Mar 08, 2013 at 08:37:00PM +0800, Peter Chen wrote:
> > On Fri, Mar 08, 2013 at 01:30:52PM +0200, Felipe Balbi wrote:
> > > this method will be used to enable or disable
> > > the charge pump.
> > >
> > > Whenever we have DRD devices, we need to be
> > > able to turn VBUS on or off whenever we want.
> > >
> > > Note that in the ideal case, this would be
> > > controlled by the ID-pin Interrupt, but not
> > > all devices have ID-pin properly routed since
> > > manufacturers can choose to save that trace
> > > if they're building a host-only product out
> > > of a DRD IP.
> > >
> > > This is also useful during debugging where
> > > we might not have the proper cable hanging
> > > around.
> > >
> > > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > > ---
> > > include/linux/usb/phy.h | 21 +++++++++++++++++++++
> > > 1 file changed, 21 insertions(+)
> > >
> > > diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> > > index b001dc3..b7c2217 100644
> > > --- a/include/linux/usb/phy.h
> > > +++ b/include/linux/usb/phy.h
> > > @@ -91,6 +91,9 @@ struct usb_phy {
> > > int (*init)(struct usb_phy *x);
> > > void (*shutdown)(struct usb_phy *x);
> > >
> > > + /* enable/disable VBUS */
> > > + int (*set_vbus)(struct usb_phy *x, int on);
> > > +
> > > /* effective for B devices, ignored for A-peripheral */
> > > int (*set_power)(struct usb_phy *x,
> > > unsigned mA);
> > > @@ -160,6 +163,24 @@ usb_phy_shutdown(struct usb_phy *x)
> > > x->shutdown(x);
> > > }
> > >
> > > +static inline int
> > > +usb_phy_vbus_on(struct usb_phy *x)
> > > +{
> > > + if (!x->set_vbus)
> > > + return 0;
> > > +
> > > + return x->set_vbus(x, true);
> > > +}
> > > +
> > > +static inline int
> > > +usb_phy_vbus_off(struct usb_phy *x)
> > > +{
> > > + if (!x->set_vbus)
> > > + return 0;
> > > +
> > > + return x->set_vbus(x, false);
> > > +}
> > > +
> >
> > What's plan for otg's set_vbus? I suggest using phy's to instead of
> > otg's.
>
> yeah, that's gotta go. Even if we're host only, we might still want to
> disable the charge pump to save some power.
>
> If you look into USB Power Delivery class, some new connectors will be
> coming to the market where there will be a switch inside the connector
> so you can know a cable is connected. That means we can keep vbus turned
> off until that switch is toggled, clearly not an OTG-specific thing ;-)
>
> Thanks for spotting that, do you wanna send a patch removing set_vbus
> from otg.h ? There are some users of that though:
>
> $ git grep -e otg_set_vbus
> arch/arm/mach-pxa/pxa3xx-ulpi.c: err = otg_set_vbus(u2d->otg->otg, 1);
> arch/arm/mach-pxa/pxa3xx-ulpi.c: otg_set_vbus(u2d->otg->otg, 0);
> drivers/usb/host/ehci-mxc.c: ret = otg_set_vbus(pdata->otg->otg, 1);
> drivers/usb/musb/musb_gadget.c: otg_set_vbus(otg, 1);
> drivers/usb/musb/omap2430.c: otg_set_vbus(otg, 1);
> drivers/usb/musb/omap2430.c: otg_set_vbus(musb->xceiv->otg, 0);
>
> I guess most are easy enough to fix by switching over to the phy
> interface. Doesn't to happen *right now* though.
>
I would like to submit patchset to clean it up, do you think
below two patches are ok:
- patch 1:
Add phy->set_vbus to where otg->set_vbus is assigned
- patch 2:
Delete otg->set_vbus, otg_set_vbus, and otg_set_vbus at otg.h
--
Best Regards,
Peter Chen
--
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
