All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
To: Neil Zhang <zhangwm@marvell.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: balbi@ti.com, gregkh@linuxfoundation.org
Subject: Re: [PATCH] usb: gadget: don't create new string_container if already exist
Date: Tue, 28 Oct 2014 14:10:27 +0100	[thread overview]
Message-ID: <544F95C3.2080306@samsung.com> (raw)
In-Reply-To: <1414496039-5655-1-git-send-email-zhangwm@marvell.com>

Hi,

W dniu 28.10.2014 o 12:33, Neil Zhang pisze:
> Don't create new usb_gadget_string_container if the current strings are
> already exist in the usb_composite_dev.
> Otherwise the ids_tab will overflow soon if we bind / unbind usb
> functions frequently like android does.

The problem you are describing does not exist in mainline kernel,
where functions are always unbound as part of the whole gadget's
unbind - regardless of whether it is a legacy gadget or configfs-composed
gadget. When the whole gadget is unbound, composite_dev_cleanup()
is called which zeroes cdev->next_string_id and frees all gadget
strings containers.

>
> Signed-off-by: Neil Zhang <zhangwm@marvell.com>
> ---
>   drivers/usb/gadget/composite.c |    6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index a8c18df..6fe3c6b 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -1183,6 +1183,12 @@ struct usb_string *usb_gstrings_attach(struct usb_composite_dev *cdev,
>   	if (!n_gstrings)
>   		return ERR_PTR(-EINVAL);
>
> +	list_for_each_entry(uc, &cdev->gstrings, list) {
> +		n_gs = get_containers_gs(uc);
> +		if (!strcmp(n_gs[0]->strings[0].s, sp[0]->strings[0].s))

To me it looks like it is a big assumption that if the first string matches,
the rest are the same, too. Isn't it?

Anyway, this solution looks more like pushing the moment when cdev->next_string_id
becomes 254 to a later time rather than preventing such a situation.

If usb_gstrings_attach() happens at function bind time, perhaps there
should be some usb_gstrings_detach() called at function unbind?

AP


  reply	other threads:[~2014-10-28 13:10 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-28 11:33 [PATCH] usb: gadget: don't create new string_container if already exist Neil Zhang
2014-10-28 13:10 ` Andrzej Pietrasiewicz [this message]
2014-11-04 11:05   ` Neil Zhang
2014-11-04 11:29     ` Andrzej Pietrasiewicz
2014-11-05 19:08     ` Felipe Balbi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=544F95C3.2080306@samsung.com \
    --to=andrzej.p@samsung.com \
    --cc=balbi@ti.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=zhangwm@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.