On Wed, Aug 13, 2014 at 01:26:07AM +0000, Peter Chen wrote:
>
> > On Mon, Aug 11, 2014 at 02:34:33PM +0200, Michael Grzeschik wrote:
> > > Hi Peter,
> > >
> > > On Mon, Aug 11, 2014 at 12:56:47AM +0000, Peter Chen wrote:
> > > > > I have seen this call is still returning EOPNOTSUPP which is also
> > > > > returned by usb_gadget_connect and usb_gadget_disconnect if the
> > > > > pullup function is not available. Should it not handle that case
> > somehow special?
> > > > >
> > > > >
> > > > > Is it still a valid condition to bailout if vbus is not active?
> > > > >
> > > >
> > > > Hi Michael,
> > > >
> > > > I did this for possible pullup dp without connection (eg load gadget
> > > > driver before vbus connect), it breaks USB spec, see 7.1.7.3 Connect
> > and Disconnect Signaling at USB 2.0 spec.
> > > >
> > > > I don't know what exactly problem you met, but current pullup dp
> > > > during loading gadget driver behavior is not suitable for webcam and
> > > > android use case even vbus is there.
> > >
> > > I still work with the uvc gadget. The code in gadget/f_uvc.c calls
> > > usb_function_deactivate. The code of uvc_function_bind will bail out
> > > in the error code path, when the usb cable is not plugged (vbus not
> > > valid) in that situation.
> >
> > I see the problem, doesn't there have problem with usb cable connected?
> > I don't see any places can avoid pull up dp at udc-core during loading
> > gadget driver.
> >
> > >
> > > When the spec defines that calling pullup is only allowed on valid
> > > vbus, we should somehow tell something different then EOPNOTSUPP so
> > > the caller and that can bail out differently.
> > >
> >
> > I will have a look for this problem
> >
>
> Does below patch can satisfy your requirement?
Yes.
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index b8125aa..cad0808 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1531,8 +1531,9 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
> {
> struct ci_hdrc *ci = container_of(_gadget, struct ci_hdrc, gadget);
>
> - if (!ci->vbus_active)
> - return -EOPNOTSUPP;
> + if (!ci->vbus_active && is_on)
> + /* See USB 2.0 Spec 7.2.1 and 7.1.5.1 */
> + return -EPERM;
>
> But long term, we should avoid it from udc-core and composite driver, since pullup dp when
> the vbus is not there breaks usb spec.
>
> http://marc.info/?l=linux-usb&m=140785265013418&w=2
This will probably better handled in the api not in every
single udc driver itself. So until we have a proper solution
we can keep the above patch.
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
