From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030759AbbD1QmO (ORCPT ); Tue, 28 Apr 2015 12:42:14 -0400 Received: from arroyo.ext.ti.com ([192.94.94.40]:37843 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030247AbbD1QmM (ORCPT ); Tue, 28 Apr 2015 12:42:12 -0400 Date: Tue, 28 Apr 2015 11:40:34 -0500 From: Felipe Balbi To: Robert Baldyga CC: , , , , , Subject: Re: [PATCH 1/2] usb: gadget: add usb_gadget_activate/deactivate functions Message-ID: <20150428164034.GH18263@saruman.tx.rr.com> Reply-To: References: <1428395513-22229-1-git-send-email-r.baldyga@samsung.com> <1428395513-22229-2-git-send-email-r.baldyga@samsung.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="svExV93C05KqedWb" Content-Disposition: inline In-Reply-To: <1428395513-22229-2-git-send-email-r.baldyga@samsung.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --svExV93C05KqedWb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Apr 07, 2015 at 10:31:52AM +0200, Robert Baldyga wrote: > These functions allows to deactivate gadget to make it not visible to > host and make it active again when gadget driver is finally ready. >=20 > They are needed to fix usb_function_activate() and usb_function_deactivat= e() > functions which currently are not working as usb_gadget_connect() is > called immediately after function bind regardless to previous calls of > usb_gadget_disconnect() function. and that's what needs to be fixed, a long time ago I wrote the patch below which I never got to finishing: commit a23800e2463ae1f4eafa7c0a15bb44afee75994f Author: Felipe Balbi Date: Thu Jul 26 14:23:44 2012 +0300 usb: gadget: let gadgets control pullup on their own =20 This is useful on gadgets that depend on userland daemons to function properly. We can delay connection to the host until userland is ready. =20 Signed-off-by: Felipe Balbi diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index 7c821de8ce3d..790ccf29f2ee 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -1784,8 +1784,9 @@ int usb_composite_probe(struct usb_composite_driver *= driver) driver->name =3D "composite"; =20 driver->gadget_driver =3D composite_driver_template; - gadget_driver =3D &driver->gadget_driver; =20 + gadget_driver =3D &driver->gadget_driver; + gadget_driver->controls_pullups =3D driver->controls_pullups; gadget_driver->function =3D (char *) driver->name; gadget_driver->driver.name =3D driver->name; gadget_driver->max_speed =3D driver->max_speed; diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c index 8a1eeb24ae6a..c0f4fca9384b 100644 --- a/drivers/usb/gadget/udc-core.c +++ b/drivers/usb/gadget/udc-core.c @@ -235,7 +235,18 @@ static void usb_gadget_remove_driver(struct usb_udc *u= dc) =20 kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); =20 - usb_gadget_disconnect(udc->gadget); + /* + * NOTICE: if gadget driver wants to control + * pullup, it needs to make sure that when + * user tries to rmmod the gadget driver, it + * will disconnect the pullups before returning + * from its ->unbind() method. + * + * We are truly trusting the gadget driver here. + */ + if (!udc->driver->controls_pullups) + usb_gadget_disconnect(udc->gadget); + udc->driver->disconnect(udc->gadget); udc->driver->unbind(udc->gadget); usb_gadget_udc_stop(udc->gadget, udc->driver); @@ -300,7 +311,18 @@ static int udc_bind_to_driver(struct usb_udc *udc, str= uct usb_gadget_driver *dri driver->unbind(udc->gadget); goto err1; } - usb_gadget_connect(udc->gadget); + + /* + * NOTICE: if gadget driver wants to control + * pullups, it needs to make sure its calls + * to usb_function_activate() and + * usb_function_deactivate() are balanced, + * otherwise gadget_driver will never enumerate. + * + * We are truly trusting the gadget driver here. + */ + if (!driver->controls_pullups) + usb_gadget_connect(udc->gadget); =20 kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); return 0; diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h index 3c671c1b37f6..7ae797c85cb9 100644 --- a/include/linux/usb/composite.h +++ b/include/linux/usb/composite.h @@ -157,6 +157,7 @@ struct usb_function { int (*get_status)(struct usb_function *); int (*func_suspend)(struct usb_function *, u8 suspend_opt); + /* private: */ /* internals */ struct list_head list; @@ -279,6 +280,8 @@ enum { * @max_speed: Highest speed the driver supports. * @needs_serial: set to 1 if the gadget needs userspace to provide * a serial number. If one is not provided, warning will be printed. + * @controls_pullups: this driver will control pullup and udc-core shouldn= 't + * enable it by default * @bind: (REQUIRED) Used to allocate resources that are shared across the * whole device, such as string IDs, and add its configurations using * @usb_add_config(). This may fail by returning a negative errno @@ -308,6 +311,7 @@ struct usb_composite_driver { struct usb_gadget_strings **strings; enum usb_device_speed max_speed; unsigned needs_serial:1; + unsigned controls_pullups:1; =20 int (*bind)(struct usb_composite_dev *cdev); int (*unbind)(struct usb_composite_dev *); diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index 32b734d88d6b..87971fa38f08 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -774,6 +774,7 @@ static inline int usb_gadget_disconnect(struct usb_gadg= et *gadget) * @suspend: Invoked on USB suspend. May be called in_interrupt. * @resume: Invoked on USB resume. May be called in_interrupt. * @driver: Driver model state for this driver. + * @controls_pullups: tells udc-core to not enable pullups by default * * Devices are disabled till a gadget driver successfully bind()s, which * means the driver will handle setup() requests needed to enumerate (and @@ -833,6 +834,8 @@ struct usb_gadget_driver { =20 /* FIXME support safe rmmod */ struct device_driver driver; + + unsigned controls_pullups:1; }; =20 =20 Together with that, I wrote: commit 0b733885e276a38cc9fa415c0977f063f9ce4d9d Author: Felipe Balbi Date: Wed Feb 6 12:34:33 2013 +0200 usb: gadget: webcam: let it control pullups =20 this gadget driver needs to make sure that we will only enumerate after its userland counterpart is up and running. =20 Signed-off-by: Felipe Balbi diff --git a/drivers/usb/gadget/webcam.c b/drivers/usb/gadget/webcam.c index 8cef1e658c29..41a4d03715bc 100644 --- a/drivers/usb/gadget/webcam.c +++ b/drivers/usb/gadget/webcam.c @@ -388,6 +388,7 @@ static __refdata struct usb_composite_driver webcam_dri= ver =3D { .max_speed =3D USB_SPEED_SUPER, .bind =3D webcam_bind, .unbind =3D webcam_unbind, + .controls_pullups =3D true, }; =20 static int __init This makes it really simple, either a gadget driver wants to control pullups, or it doesn't. For those who wish to control pullups (for whatever reason) we rely completely on the gadget telling us it's ok to connect pullups by means of usb_function_activate()/deactivate(), for all others, we just connect straight away. cheers --=20 balbi --svExV93C05KqedWb Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVP7gCAAoJEIaOsuA1yqREBRgP/1p9Fe+5t8CRji5vEE6sCvGA 6O+Bi32Lao68+Y8sTzJkdWCa6NwjmGNB+eiRUlBpF3BuYtP13TgnvnyoaHXni3mZ OHV4t3OvOF+S6Ayz+Baj3PsUlxvtfGFw43b9lIL03cXahZVL6lW5FqfJ4qInyFzr Ndmu7sTskEK//d2RjpuJ7nj0cbZNJVMNTrXAHAOuo5YWhTs98FMu4FGD67FCNxX7 Zfuoyq0CuUOhVUwVJo5tbiX7jfyMaUmv1QyAQZC5gbGBgqMk7xsctex3VlK/JroW pxA8BXxJb6n/vlnDkM9qc83wgkodJdO7I0yBk1t25KjeWVE31V8sEGHTh5eGmYDx lf9xypEeW17PMjrAhCSM+SOStAGMKsAw5UYQuvjhX7VD8gKZu+cNgsrehOCaALEn SxOapDxXzSct7yZbAvVFgm4VwKLlSfzqPpRPMoV+cr+jb8JeeaoZMjuSJTHhZasL RIb2SiMhSX4lkTqB/EWS8ZKZ1v6900NBqDvokNROMLiSHpkaNmTr+EgsGct7Cwg0 FSIyyV+UDO1E1XjWEvEjOLeyzqkiTOVqmbbW6RBCLzAefxYoifsQXo+TMD48xCkT VqkgPf0jppuoc0u3RNjRD7NSIQDoZgnyR9VnJG7ugH6EZ5qcraR0hmP/Kob7gUdB hmEPeL5OJ1PE1S81a+s1 =F06l -----END PGP SIGNATURE----- --svExV93C05KqedWb--