[PATCH v2 0/5] usb: gadget: miscellaneous fixes

[PATCH v2 0/5] usb: gadget: miscellaneous fixes

[Robert Baldyga]

Hello,

This patch set contains few small bugfixes found in usb gadget functions
and UDC drivers. The most important is the [1] as it fixes bug causing
BUG_ON() in f_fs driver. Remaining patches contain minor fixes.

Best regards,
Robert Baldyga

Changelog:

v2:
- Addressed comments from Dan Carpenter and Sergei Shtylyov.

v1: https://lkml.org/lkml/2015/7/7/445

Robert Baldyga (5):
  usb: gadget: ffs: call functionfs_unbind() if _ffs_func_bind() fails
  usb: gadget: midi: avoid redundant f_midi_set_alt() call
  usb: isp1760: udc: add missing usb_ep_set_maxpacket_limit()
  staging: emxx_udc: add missing usb_ep_set_maxpacket_limit()
  usb: gadget: atmel_usba_udc: add missing ret value check

 drivers/staging/emxx_udc/emxx_udc.c     | 3 ++-
 drivers/usb/gadget/function/f_fs.c      | 9 ++++++++-
 drivers/usb/gadget/function/f_midi.c    | 4 ++++
 drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++++
 drivers/usb/isp1760/isp1760-udc.c       | 4 ++--
 5 files changed, 20 insertions(+), 4 deletions(-)

-- 
1.9.1

[PATCH v2 1/5] usb: gadget: ffs: call functionfs_unbind() if _ffs_func_bind() fails

[Robert Baldyga]

Function ffs_do_functionfs_bind() calls functionfs_bind() which allocates
usb request and increments refcounts. These things needs to be cleaned
up by if further steps of initialization fail by calling functionfs_unbind().

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_fs.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 6e7be91..6516187 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -2897,11 +2897,18 @@ static int ffs_func_bind(struct usb_configuration *c,
 			 struct usb_function *f)
 {
 	struct f_fs_opts *ffs_opts = ffs_do_functionfs_bind(f, c);
+	struct ffs_function *func = ffs_func_from_usb(f);
+	int ret;
 
 	if (IS_ERR(ffs_opts))
 		return PTR_ERR(ffs_opts);
 
-	return _ffs_func_bind(c, f);
+	ret = _ffs_func_bind(c, f);
+	if (ret)
+		if(!--ffs_opts->refcnt)
+			functionfs_unbind(func->ffs);
+
+	return ret;
 }
 
 
-- 
1.9.1

[PATCH v2 2/5] usb: gadget: midi: avoid redundant f_midi_set_alt() call

[Robert Baldyga]

Function midi registers two interfaces with single set_alt() function
which means that f_midi_set_alt() is called twice when configuration
is set. That means that endpoint initialization and ep request allocation
is done two times. To avoid this problem we do such things only once,
for interface number 1 (MIDI Streaming interface).

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_midi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index 6316aa5..4cef222 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -329,6 +329,10 @@ static int f_midi_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 	unsigned i;
 	int err;
 
+	/* For Control Device interface we do nothing */
+	if (intf == 0)
+		return 0;
+
 	err = f_midi_start_ep(midi, f, midi->in_ep);
 	if (err)
 		return err;
-- 
1.9.1

[PATCH v2 3/5] usb: isp1760: udc: add missing usb_ep_set_maxpacket_limit()

[Robert Baldyga]

Since maxpacket_limit was introduced all UDC drivers should use
usb_ep_set_maxpacket_limit() function instead of setting maxpacket value
manually. ep.maxpacket_limit contains actual maximum maxpacket value
supported by hardware which is needed by epautoconf.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/isp1760/isp1760-udc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/isp1760/isp1760-udc.c b/drivers/usb/isp1760/isp1760-udc.c
index 18ebf5b1..3699962 100644
--- a/drivers/usb/isp1760/isp1760-udc.c
+++ b/drivers/usb/isp1760/isp1760-udc.c
@@ -1382,11 +1382,11 @@ static void isp1760_udc_init_eps(struct isp1760_udc *udc)
 		 * This fits in the 8kB FIFO without double-buffering.
 		 */
 		if (ep_num == 0) {
-			ep->ep.maxpacket = 64;
+			usb_ep_set_maxpacket_limit(&ep->ep, 64);
 			ep->maxpacket = 64;
 			udc->gadget.ep0 = &ep->ep;
 		} else {
-			ep->ep.maxpacket = 512;
+			usb_ep_set_maxpacket_limit(&ep->ep, 512);
 			ep->maxpacket = 0;
 			list_add_tail(&ep->ep.ep_list, &udc->gadget.ep_list);
 		}
-- 
1.9.1

[PATCH v2 4/5] staging: emxx_udc: add missing usb_ep_set_maxpacket_limit()

[Robert Baldyga]

Since maxpacket_limit was introduced all UDC drivers should use
usb_ep_set_maxpacket_limit() function instead of setting maxpacket value
manually. ep.maxpacket_limit contains actual maximum maxpacket value
supported by hardware which is needed by epautoconf.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/staging/emxx_udc/emxx_udc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
index 4178d96..3b7aa36 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -3203,7 +3203,8 @@ static void __init nbu2ss_drv_ep_init(struct nbu2ss_udc *udc)
 		ep->ep.name = gp_ep_name[i];
 		ep->ep.ops = &nbu2ss_ep_ops;
 
-		ep->ep.maxpacket = (i == 0 ? EP0_PACKETSIZE : EP_PACKETSIZE);
+		usb_ep_set_maxpacket_limit(&ep->ep,
+				i == 0 ? EP0_PACKETSIZE : EP_PACKETSIZE);
 
 		list_add_tail(&ep->ep.ep_list, &udc->gadget.ep_list);
 		INIT_LIST_HEAD(&ep->queue);
-- 
1.9.1

[PATCH v2 5/5] usb: gadget: atmel_usba_udc: add missing ret value check

[Robert Baldyga]

Add missing return value check. In case of error print debug message
and return error code.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 4095cce0..37d414e 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1989,6 +1989,10 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
 		ep->can_isoc = of_property_read_bool(pp, "atmel,can-isoc");
 
 		ret = of_property_read_string(pp, "name", &name);
+		if (ret) {
+			dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
+			goto err;
+		}
 		ep->ep.name = name;
 
 		ep->ep_regs = udc->regs + USBA_EPT_BASE(i);
-- 
1.9.1

Re: [PATCH v2 1/5] usb: gadget: ffs: call functionfs_unbind() if _ffs_func_bind() fails

[Sergei Shtylyov]

Hello.

On 7/8/2015 12:05 PM, Robert Baldyga wrote:

> Function ffs_do_functionfs_bind() calls functionfs_bind() which allocates
> usb request and increments refcounts. These things needs to be cleaned
> up by if further steps of initialization fail by calling functionfs_unbind().

> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> ---
>   drivers/usb/gadget/function/f_fs.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)

> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index 6e7be91..6516187 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -2897,11 +2897,18 @@ static int ffs_func_bind(struct usb_configuration *c,
>   			 struct usb_function *f)
>   {
>   	struct f_fs_opts *ffs_opts = ffs_do_functionfs_bind(f, c);
> +	struct ffs_function *func = ffs_func_from_usb(f);
> +	int ret;
>
>   	if (IS_ERR(ffs_opts))
>   		return PTR_ERR(ffs_opts);
>
> -	return _ffs_func_bind(c, f);
> +	ret = _ffs_func_bind(c, f);
> +	if (ret)
> +		if(!--ffs_opts->refcnt)
> +			functionfs_unbind(func->ffs);

    Hum, I think you need {} around the outer *if* clause. Either that, or 
just fold two *if* statments together.

> +
> +	return ret;
>   }
[...]

WBR, Sergei

Re: [PATCH v2 1/5] usb: gadget: ffs: call functionfs_unbind() if _ffs_func_bind() fails

[Michal Nazarewicz]

On Wed, Jul 08 2015, Robert Baldyga wrote:
> Function ffs_do_functionfs_bind() calls functionfs_bind() which allocates
> usb request and increments refcounts. These things needs to be cleaned
> up by if further steps of initialization fail by calling functionfs_unbind().
>
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> ---
>  drivers/usb/gadget/function/f_fs.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index 6e7be91..6516187 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -2897,11 +2897,18 @@ static int ffs_func_bind(struct usb_configuration *c,
>  			 struct usb_function *f)
>  {
>  	struct f_fs_opts *ffs_opts = ffs_do_functionfs_bind(f, c);
> +	struct ffs_function *func = ffs_func_from_usb(f);
> +	int ret;
>  
>  	if (IS_ERR(ffs_opts))
>  		return PTR_ERR(ffs_opts);
>  
> -	return _ffs_func_bind(c, f);
> +	ret = _ffs_func_bind(c, f);
> +	if (ret)
> +		if(!--ffs_opts->refcnt)
> +			functionfs_unbind(func->ffs);
> +
> +	return ret;
>  }
>  
>  
> -- 
> 1.9.1
>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

Re: [PATCH v2 2/5] usb: gadget: midi: avoid redundant f_midi_set_alt() call

[Michal Nazarewicz]

On Wed, Jul 08 2015, Robert Baldyga wrote:
> Function midi registers two interfaces with single set_alt() function
> which means that f_midi_set_alt() is called twice when configuration
> is set. That means that endpoint initialization and ep request allocation
> is done two times. To avoid this problem we do such things only once,
> for interface number 1 (MIDI Streaming interface).
>
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> ---
>  drivers/usb/gadget/function/f_midi.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index 6316aa5..4cef222 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -329,6 +329,10 @@ static int f_midi_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
>  	unsigned i;
>  	int err;
>  
> +	/* For Control Device interface we do nothing */
> +	if (intf == 0)
> +		return 0;
> +
>  	err = f_midi_start_ep(midi, f, midi->in_ep);
>  	if (err)
>  		return err;
> -- 
> 1.9.1
>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

Re: [PATCH v2 3/5] usb: isp1760: udc: add missing usb_ep_set_maxpacket_limit()

[Michal Nazarewicz]

On Wed, Jul 08 2015, Robert Baldyga wrote:
> Since maxpacket_limit was introduced all UDC drivers should use
> usb_ep_set_maxpacket_limit() function instead of setting maxpacket value
> manually. ep.maxpacket_limit contains actual maximum maxpacket value
> supported by hardware which is needed by epautoconf.
>
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> ---
>  drivers/usb/isp1760/isp1760-udc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/isp1760/isp1760-udc.c b/drivers/usb/isp1760/isp1760-udc.c
> index 18ebf5b1..3699962 100644
> --- a/drivers/usb/isp1760/isp1760-udc.c
> +++ b/drivers/usb/isp1760/isp1760-udc.c
> @@ -1382,11 +1382,11 @@ static void isp1760_udc_init_eps(struct isp1760_udc *udc)
>  		 * This fits in the 8kB FIFO without double-buffering.
>  		 */
>  		if (ep_num == 0) {
> -			ep->ep.maxpacket = 64;
> +			usb_ep_set_maxpacket_limit(&ep->ep, 64);
>  			ep->maxpacket = 64;
>  			udc->gadget.ep0 = &ep->ep;
>  		} else {
> -			ep->ep.maxpacket = 512;
> +			usb_ep_set_maxpacket_limit(&ep->ep, 512);
>  			ep->maxpacket = 0;
>  			list_add_tail(&ep->ep.ep_list, &udc->gadget.ep_list);
>  		}
> -- 
> 1.9.1
>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

Re: [PATCH v2 4/5] staging: emxx_udc: add missing usb_ep_set_maxpacket_limit()

[Michal Nazarewicz]

On Wed, Jul 08 2015, Robert Baldyga wrote:
> Since maxpacket_limit was introduced all UDC drivers should use
> usb_ep_set_maxpacket_limit() function instead of setting maxpacket value
> manually. ep.maxpacket_limit contains actual maximum maxpacket value
> supported by hardware which is needed by epautoconf.
>
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> ---
>  drivers/staging/emxx_udc/emxx_udc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
> index 4178d96..3b7aa36 100644
> --- a/drivers/staging/emxx_udc/emxx_udc.c
> +++ b/drivers/staging/emxx_udc/emxx_udc.c
> @@ -3203,7 +3203,8 @@ static void __init nbu2ss_drv_ep_init(struct nbu2ss_udc *udc)
>  		ep->ep.name = gp_ep_name[i];
>  		ep->ep.ops = &nbu2ss_ep_ops;
>  
> -		ep->ep.maxpacket = (i == 0 ? EP0_PACKETSIZE : EP_PACKETSIZE);
> +		usb_ep_set_maxpacket_limit(&ep->ep,
> +				i == 0 ? EP0_PACKETSIZE : EP_PACKETSIZE);

I would break line just after ( like so:

+		usb_ep_set_maxpacket_limit(
+				&ep->ep, i ? EP_PACKETSIZE : EP0_PACKETSIZE);

>  
>  		list_add_tail(&ep->ep.ep_list, &udc->gadget.ep_list);
>  		INIT_LIST_HEAD(&ep->queue);
> -- 
> 1.9.1
>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

Re: [PATCH v2 5/5] usb: gadget: atmel_usba_udc: add missing ret value check

[Michal Nazarewicz]

On Wed, Jul 08 2015, Robert Baldyga wrote:
> Add missing return value check. In case of error print debug message
> and return error code.
>
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> ---
>  drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index 4095cce0..37d414e 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -1989,6 +1989,10 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
>  		ep->can_isoc = of_property_read_bool(pp, "atmel,can-isoc");
>  
>  		ret = of_property_read_string(pp, "name", &name);
> +		if (ret) {
> +			dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
> +			goto err;
> +		}
>  		ep->ep.name = name;
>  
>  		ep->ep_regs = udc->regs + USBA_EPT_BASE(i);
> -- 
> 1.9.1
>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--