On Tue, Nov 20, 2012 at 08:52:04AM +0100, Sascha Hauer wrote:
> On Tue, Nov 20, 2012 at 01:54:48PM +0800, Peter Chen wrote:
> > On Mon, Nov 19, 2012 at 08:17:34PM +0100, Sascha Hauer wrote:
> > > Hi Peter,
> > >
> > > > @@ -139,6 +139,7 @@ struct ci13xxx {
> > > > enum ci_role role;
> > > > bool is_otg;
> > > > struct work_struct work;
> > > > + struct delayed_work dwork;
> > > > struct workqueue_struct *wq;
> > > >
> > > > struct dma_pool *qh_pool;
> > > > @@ -164,6 +165,11 @@ struct ci13xxx {
> > > > bool global_phy;
> > > > struct usb_phy *transceiver;
> > > > struct usb_hcd *hcd;
> > > > + /* events handled at ci_role_work */
> > > > +#define ID 0
> > > > +#define B_SESS_VALID 1
> > > > + unsigned long events;
> > > > + struct usb_otg otg;
> > >
> > > I looked into implementing ULPI support for the chipidea driver. This
> > > does not integrate very well with having a struct usb_otg here. Instead
> > > it should be a pointer. Look into drivers/usb/otg/ulpi.c. This
> > > allocates the struct usb_otg itself and we have to feed the pointer into
> > > struct ci13xxx.
> >
> > We discussed before that the otg is not related to phy.
>
> Maybe the real problem is that the usb_otg instead of the phy provides
> the set_vbus callback. It's a bit strange that the phy has the
> set_power callback to draw current and the otg has the set_vbus callback
> to provide vbus.
>
> Moving set_vbus to the phy would solve things, then the ulpi driver
> would have no business creating a struct usb_otg and this could be
> private to the chipidea driver. We would also have the possibility to
> provide a set_vbus function without (needed for ulpi even for host-only
> mode) without having a struct usb_otg.
Let's see the user case for set_vbus and set_power
set_vbus:
For host: set_vbus(1) at module probe, set_vbus(0) at module remove.
For otg: set_vbus(1) when switch to host mode, set_vbus(0) when switch
to device mode.
set_power:
Only uses at device mode to indicate or tell pmic how much current it can
draw from the vbus?
eg:
connect but unconfigure, 100mA
configured: 500mA
suspend: <2mA
So, both set_vbus and set_power are for usb itself, not for otg, neither for
phy. But now, we need one thing to stand for usb, every usb has its phy, so
we can move things stands for usb to struct usb_phy.
Felipe, what's your opinion?
>
> Sascha
>
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>
--
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
