Re: [PATCH] usb: chipidea: udc: first start device on pullup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 12, 2013 at 08:36:47AM +0100, Michael Grzeschik wrote:
> 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.

Yes, In fact, there are two places you need to modify:

- hw_device_state at chipidea/udc.c
- udc_bind_to_driver at gadget/udc-core.c (it will call .pullup,
if vbus is there, the usbcmd.rs will still be set)

Peter

> 
> > > > 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 |
> 

-- 

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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux