On Monday 27 July 2009, Felipe Balbi wrote:
> gadgets with multiple configurations will have problems with
> deactivations since they will register function_driver * configurations
> instances of each function driver, which will make
> cdev->deactivations to never become zero as it should.
I don't follow. Surely if there is *any* function which isn't
ready to go, in any configuration, the device then must not be
activated. The only time it's safe to activate/reactivate the
device is if every function, in every configuration, is ready.
Because the first thing the host can do is activate the config
with that un-ready configuration, and then try to use the function
which is not ready...
> Moving deactivations to usb_configuration in order seems to
> be the right fix to prevent the failure where gadget won't
> ever connect to usb bus.
I'd have said the issue is that some function isn't properly
listing itself as ready-to-go ...
- Dave
> Cc: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxx>
> Cc: Bryan Wu <cooloney@xxxxxxxxxx>
> Cc: Robert Jarzmik <robert.jarzmik@xxxxxxx>
> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxx>
> ---
> drivers/usb/gadget/composite.c | 16 +++++++++-------
> include/linux/usb/composite.h | 5 -----
> 2 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 59e8523..3e3380d 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -148,16 +148,17 @@ done:
> */
> int usb_function_deactivate(struct usb_function *function)
> {
> - struct usb_composite_dev *cdev = function->config->cdev;
> + struct usb_configuration *config = function->config;
> + struct usb_composite_dev *cdev = config->cdev;
> unsigned long flags;
> int status = 0;
>
> spin_lock_irqsave(&cdev->lock, flags);
>
> - if (cdev->deactivations == 0)
> + if (config->deactivations == 0)
> status = usb_gadget_disconnect(cdev->gadget);
> if (status == 0)
> - cdev->deactivations++;
> + config->deactivations++;
>
> spin_unlock_irqrestore(&cdev->lock, flags);
> return status;
> @@ -175,16 +176,17 @@ int usb_function_deactivate(struct usb_function *function)
> */
> int usb_function_activate(struct usb_function *function)
> {
> - struct usb_composite_dev *cdev = function->config->cdev;
> + struct usb_configuration *config = function->config;
> + struct usb_composite_dev *cdev = config->cdev;
> int status = 0;
>
> spin_lock(&cdev->lock);
>
> - if (WARN_ON(cdev->deactivations == 0))
> + if (WARN_ON(config->deactivations == 0))
> status = -EINVAL;
> else {
> - cdev->deactivations--;
> - if (cdev->deactivations == 0)
> + config->deactivations--;
> + if (config->deactivations == 0)
> status = usb_gadget_connect(cdev->gadget);
> }
>
> diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
> index 4f6bb3d..7e2c119 100644
> --- a/include/linux/usb/composite.h
> +++ b/include/linux/usb/composite.h
> @@ -330,11 +330,6 @@ struct usb_composite_dev {
> struct usb_composite_driver *driver;
> u8 next_string_id;
>
> - /* the gadget driver won't enable the data pullup
> - * while the deactivation count is nonzero.
> - */
> - unsigned deactivations;
> -
> /* protects at least deactivation count */
> spinlock_t lock;
> };
> --
> 1.6.3.3.385.g60647
>
>
--
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
