Hi Peter,
On Tue, Nov 12, 2013 at 10:59:09AM +0800, Peter Chen wrote:
> On Mon, Nov 11, 2013 at 04:37:26PM +0100, Michael Grzeschik wrote:
> > On Mon, Nov 11, 2013 at 09:42:35PM +0800, Peter Chen wrote:
> > > On Mon, Nov 11, 2013 at 12:39:30PM +0100, Michael Grzeschik wrote:
> > > > Don't pullup the resistors on hw_device_state. The gadget framework has
> > > > the prepared callback for this. This is necessary if we use gadgets
> > > > which need to be enabled after an userspace application got prepared, or
> > > > other delayed conditiions have passed.
> > > >
> > > > Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> > > > ---
> > > > drivers/usb/chipidea/udc.c | 2 --
> > > > 1 file changed, 2 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > > > index b34c819..976153f 100644
> > > > --- a/drivers/usb/chipidea/udc.c
> > > > +++ b/drivers/usb/chipidea/udc.c
> > > > @@ -84,10 +84,8 @@ static int hw_device_state(struct ci_hdrc *ci, u32 dma)
> > > > /* interrupt, error, port change, reset, sleep/suspend */
> > > > hw_write(ci, OP_USBINTR, ~0,
> > > > USBi_UI|USBi_UEI|USBi_PCI|USBi_URI|USBi_SLI);
> > > > - hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
> > > > } else {
> > > > hw_write(ci, OP_USBINTR, ~0, 0);
> > > > - hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
> > > > }
> > > > return 0;
> > > > }
> > >
> > > Hi Michael, you submitted a similar patch before to fix uvc problem,
> > > but you can't simply delete it, it will cause the udc can't work
> > > if we load gadget first, then plug in the usb cable.
> >
> > Yes, you also mentioned this the last time. But here, when the
> > gadget was loaded first and plugged in afterwarts works as well.
> >
>
> It can't work at my place, how it can work? There is no place to
> set usbcmd.rs, you can read the register usbcmd after load gadget module.
>
> > Did you see when the pullup function get called? I did also send a
> > patch that is changing the pullup call inside the gadget drivers.
> >
>
> Current, usbcmd.rs is only set by two places at udc.c
>
> - hw_device_state
> - ci_duc_pullup (if vbus is there)
>
> If you delete it at hw_device_state, the usb_gadget_connect can't
> set it without vbus. I don't know why it can work at your place
> since there is no place to set USBCMD_RS if we load gadget first, then
> plug in usb cable.
>
> Besides, if the port support dual role switch, if the gadget is loaded
> first, then do peripheral->host and host->peripheral,(the usbcmd.rs is cleared
> at that time), then connect vbus, no place to pull up dp.
>
I checked it again. Yes we miss the RS toggle in vbus_session with that
patch. Sorry for the confusion. But we somehow need to miss the first RS
toggle in ci_udc_start. How about givin hw_device_state an parameter for
that.
> > > To fix it, it is better mark that gadget as defer_connect, and
> > > introduce such condition at udc driver.
> >
> > No, I don't see why this needs to be specially handled inside this
> > driver. We have the framework for this. USBCMD_RS does pullup the
> > DP resistor.
> >
> > The datasheet says, that we have to ensure that the device needs to be
> > properly setup before touching this bit. As the framework does call
> > usb_gadget_connect after the gadget has started and connected to the udc
> > this should be the case.
> >
>
> We all hope all things can be done at udc core, but it still has lots
> of work to do, we had a discussion it before, see below thread:
>
> http://marc.info/?l=linux-usb&m=136335043303602&w=2
I did miss that thread.
Thanks,
Michael
--
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 |
--
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
