[U-Boot] [PATCH 0/8] USB: gadget: fix porting bugs

[U-Boot] [PATCH 0/8] USB: gadget: fix porting bugs

[Vitaly Kuzmichev]

1) Restuct Makefile to unable usage old and new stacks simultaneously
2) Add lost 'qmult' definition for High Speed devices
3) Take debug printout macros back and make them more useful
4) Fix in_ep, out_ep, ep0 and status_ep confusions
5) Replace 'strcpy' by 'strlcpy'
6) Fix possible oops on stat_req->buf initialization
7) 2 updates backported from linux-2.6.git

David Brownell (1):
  USB: gadget: ethernet error path potential oops fix

Julia Lawall (1):
  USB: gadget: change simple_strtol to simple_strtoul

Vitaly Kuzmichev (6):
  USB-CDC: Restuct USB gadget Makefile
  USB-CDC: Add lost 'qmult' definition
  USB-CDC: Linux-like debug printout
  USB-CDC: Correct freeing usb requests
  USB-CDC: Replace 'strcpy' by 'strlcpy'
  USB-CDC: Correct stat_req initialization

 drivers/usb/gadget/Makefile     |    9 +++-
 drivers/usb/gadget/epautoconf.c |    2 +-
 drivers/usb/gadget/ether.c      |   95 ++++++++++++++++++++++-----------------
 3 files changed, 61 insertions(+), 45 deletions(-)

[U-Boot] [PATCH 1/8] USB-CDC: Restuct USB gadget Makefile

[Vitaly Kuzmichev]

Prohibit simultaneous usage of both old and new gadget stacks and
allow UDC drivers to be dependent on CONFIG_USB_ETHER.

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/Makefile |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 27e7f40..05aa213 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -25,6 +25,11 @@ include $(TOPDIR)/config.mk
 
 LIB	:= $(obj)libusb_gadget.a
 
+# new USB gadget layer dependencies
+ifdef CONFIG_USB_ETHER
+COBJS-y += ether.o epautoconf.o config.o usbstring.o
+COBJS-$(CONFIG_USB_GADGET_AT91) += at91_udc.o
+else
 # Devices not related to the new gadget layer depend on CONFIG_USB_DEVICE
 ifdef CONFIG_USB_DEVICE
 COBJS-y += core.o
@@ -35,9 +40,7 @@ COBJS-$(CONFIG_MPC885_FAMILY) += mpc8xx_udc.o
 COBJS-$(CONFIG_PXA27X) += pxa27x_udc.o
 COBJS-$(CONFIG_SPEARUDC) += spr_udc.o
 endif
-# new USB gadget layer dependencies
-COBJS-$(CONFIG_USB_ETHER) += ether.o epautoconf.o config.o usbstring.o
-COBJS-$(CONFIG_USB_GADGET_AT91) += at91_udc.o
+endif
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
-- 
1.7.1.1

[U-Boot] [PATCH 1/8] USB-CDC: Restuct USB gadget Makefile

[Remy Bohmer]

Hi,

2010/8/12 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
> Prohibit simultaneous usage of both old and new gadget stacks and
> allow UDC drivers to be dependent on CONFIG_USB_ETHER.
>
> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> ---
> ?drivers/usb/gadget/Makefile | ? ?9 ++++++---
> ?1 files changed, 6 insertions(+), 3 deletions(-)

Applied to u-boot-usb/cdc branch.
Thanks.

Remy

[U-Boot] [PATCH 2/8] USB-CDC: Add lost 'qmult' definition

[Vitaly Kuzmichev]

Add lost 'qmult' definition for High Speed devices and make it
configurable through CONFIG_USB_ETH_QMULT.

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/ether.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 3d871fa..a07738f 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -140,6 +140,12 @@ static inline int is_cdc(struct eth_dev *dev)
 #ifdef CONFIG_USB_GADGET_DUALSPEED
 #define	DEVSPEED	USB_SPEED_HIGH
 
+#ifdef CONFIG_USB_ETH_QMULT
+#define qmult CONFIG_USB_ETH_QMULT
+#else
+#define qmult 5
+#endif
+
 /* for dual-speed hardware, use deeper queues at highspeed */
 #define qlen(gadget) \
 	(DEFAULT_QLEN*((gadget->speed == USB_SPEED_HIGH) ? qmult : 1))
-- 
1.7.1.1

[U-Boot] [PATCH 2/8] USB-CDC: Add lost 'qmult' definition

[Remy Bohmer]

Hi,

2010/8/12 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
> Add lost 'qmult' definition for High Speed devices and make it
> configurable through CONFIG_USB_ETH_QMULT.
>
> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> ---
> ?drivers/usb/gadget/ether.c | ? ?6 ++++++
> ?1 files changed, 6 insertions(+), 0 deletions(-)

Applied to u-boot-usb/cdc branch.
Thanks.

Remy

[U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout

[Vitaly Kuzmichev]

Take debug printout macros back from linux-2.6.27 and make them more
useful and more compatible.

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/ether.c |   65 +++++++++++++++++++++++---------------------
 1 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index a07738f..b6f5f4d 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -37,8 +37,10 @@
 #define dev_err(x, stuff...) printf(stuff)
 #define dev_dbg dev_err
 #define dev_warn dev_err
-#define DEBUG dev_err
-#define VDEBUG DEBUG
+#define WARN INFO
+#define ERROR INFO
+#define DEBUG INFO
+#define VDEBUG dprintf
 #define atomic_read
 extern struct platform_data brd;
 #define spin_lock(x)
@@ -769,7 +771,7 @@ set_ether_config (struct eth_dev *dev, gfp_t gfp_flags)
 
 		result = usb_ep_enable (dev->status_ep, dev->status);
 		if (result != 0) {
-			printf ("enable %s --> %d\n",
+			DEBUG (dev, "enable %s --> %d\n",
 				dev->status_ep->name, result);
 			goto done;
 		}
@@ -789,14 +791,14 @@ set_ether_config (struct eth_dev *dev, gfp_t gfp_flags)
 	if (!cdc_active(dev)) {
 		result = usb_ep_enable (dev->in_ep, dev->in);
 		if (result != 0) {
-			printf ("enable %s --> %d\n",
+			DEBUG(dev, "enable %s --> %d\n",
 				dev->in_ep->name, result);
 			goto done;
 		}
 
 		result = usb_ep_enable (dev->out_ep, dev->out);
 		if (result != 0) {
-			printf ("enable %s --> %d\n",
+			DEBUG (dev, "enable %s --> %d\n",
 				dev->out_ep->name, result);
 			goto done;
 		}
@@ -827,6 +829,8 @@ static void eth_reset_config (struct eth_dev *dev)
 	if (dev->config == 0)
 		return;
 
+	DEBUG (dev, "%s\n", __func__);
+
 	/* disable endpoints, forcing (synchronous) completion of
 	 * pending i/o.  then free the requests.
 	 */
@@ -941,17 +945,17 @@ static void eth_status_complete (struct usb_ep *ep, struct usb_request *req)
 
 		req->length = STATUS_BYTECOUNT;
 		value = usb_ep_queue (ep, req, GFP_ATOMIC);
-		dprintf ("send SPEED_CHANGE --> %d\n", value);
+		DEBUG (dev, "send SPEED_CHANGE --> %d\n", value);
 		if (value == 0)
 			return;
 	} else if (value != -ECONNRESET) {
-		dprintf("event %02x --> %d\n",
+		DEBUG (dev, "event %02x --> %d\n",
 			event->bNotificationType, value);
 		if (event->bNotificationType==
 				USB_CDC_NOTIFY_SPEED_CHANGE)
 		{
 			l_ethdev.network_started=1;
-			printf("USB network up!\n");
+			INFO(&l_ethdev, "USB network up!\n");
 		}
 	}
 	req->context = NULL;
@@ -991,7 +995,7 @@ static void issue_start_status (struct eth_dev *dev)
 
 	value = usb_ep_queue (dev->status_ep, req, GFP_ATOMIC);
 	if (value < 0)
-		printf ("status buf queue --> %d\n", value);
+		DEBUG (dev, "status buf queue --> %d\n", value);
 }
 
 #endif
@@ -1001,7 +1005,7 @@ static void issue_start_status (struct eth_dev *dev)
 static void eth_setup_complete (struct usb_ep *ep, struct usb_request *req)
 {
 	if (req->status || req->actual != req->length)
-		dprintf  (/*(struct eth_dev *) ep->driver_data*/
+		DEBUG ((struct eth_dev *) ep->driver_data,
 				"setup complete --> %d, %d/%d\n",
 				req->status, req->actual, req->length);
 }
@@ -1029,7 +1033,7 @@ eth_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 	 * while config change events may enable network traffic.
 	 */
 
-	dprintf("eth_setup:...\n");
+	VDEBUG(dev, "%s\n", __func__);
 
 	req->complete = eth_setup_complete;
 	switch (ctrl->bRequest) {
@@ -1179,7 +1183,7 @@ done_set_intf:
 				|| wLength != 0
 				|| wIndex > 1)
 			break;
-		printf ("packet filter %02x\n", wValue);
+		DEBUG (dev, "packet filter %02x\n", wValue);
 		dev->cdc_filter = wValue;
 		value = 0;
 		break;
@@ -1194,7 +1198,7 @@ done_set_intf:
 #endif /* DEV_CONFIG_CDC */
 
 	default:
-		printf (
+		VDEBUG (dev,
 			"unknown control req%02x.%02x v%04x i%04x l%d\n",
 			ctrl->bRequestType, ctrl->bRequest,
 			wValue, wIndex, wLength);
@@ -1202,7 +1206,7 @@ done_set_intf:
 
 	/* respond with data transfer before status phase? */
 	if (value >= 0) {
-		dprintf("respond with data transfer before status phase\n");
+		DEBUG(dev, "respond with data transfer before status phase\n");
 		req->length = value;
 		req->zero = value < wLength
 				&& (value % gadget->ep0->maxpacket) == 0;
@@ -1237,7 +1241,7 @@ static int rx_submit ( struct eth_dev *dev, struct usb_request *req, \
 	 * byte off the end (to force hardware errors on overflow).
 	 */
 
-	dprintf("%s\n", __func__);
+	VDEBUG(dev, "%s\n", __func__);
 
 	size = (ETHER_HDR_SIZE + dev->mtu + RX_EXTRA);
 	size += dev->out_ep->maxpacket - 1;
@@ -1255,7 +1259,7 @@ static int rx_submit ( struct eth_dev *dev, struct usb_request *req, \
 	retval = usb_ep_queue (dev->out_ep, req, gfp_flags);
 
 	if (retval) {
-		dprintf ("rx submit --> %d\n", retval);
+		DEBUG (dev, "rx submit --> %d\n", retval);
 	}
 	return retval;
 }
@@ -1265,8 +1269,7 @@ static void rx_complete (struct usb_ep *ep, struct usb_request *req)
 {
 	struct eth_dev	*dev = ep->driver_data;
 
-	dprintf("%s\n", __func__);
-	dprintf("rx status %d\n", req->status);
+	VDEBUG(dev, "%s: status %d\n", __func__, req->status);
 
 	packet_received=1;
 
@@ -1298,7 +1301,7 @@ fail:
 
 static void tx_complete (struct usb_ep *ep, struct usb_request *req)
 {
-	dprintf("%s, status: %s\n", __func__,(req->status) ? "failed":"ok");
+	VDEBUG(ep->driver_data, "%s: status %s\n", __func__, (req->status)?"failed":"ok");
 	packet_sent=1;
 }
 
@@ -1427,7 +1430,7 @@ static void eth_unbind (struct usb_gadget *gadget)
 {
 	struct eth_dev *dev = get_gadget_data (gadget);
 
-	printf("eth_unbind:...\n");
+	DEBUG (dev, "%s...\n", __func__);
 
 	if (dev->stat_req) {
 		usb_ep_free_request (dev->status_ep, dev->stat_req);
@@ -1768,7 +1771,7 @@ static int usb_eth_init(struct eth_device* netdev, bd_t* bd)
 	unsigned long timeout = USB_CONNECT_TIMEOUT;
 
 	if (!netdev) {
-		printf("ERROR: received NULL ptr\n");
+		ERROR(dev, "ERROR: received NULL ptr\n");
 		goto fail;
 	}
 
@@ -1790,7 +1793,7 @@ static int usb_eth_init(struct eth_device* netdev, bd_t* bd)
 	{
 		/* Handle control-c and timeouts */
 		if (ctrlc() || (get_timer(ts) > timeout)) {
-			printf("The remote end did not respond in time.\n");
+			ERROR(dev, "The remote end did not respond in time.\n");
 			goto fail;
 		}
 		usb_gadget_handle_interrupts();
@@ -1806,9 +1809,9 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le
 {
 	int			retval;
 	struct usb_request	*req = NULL;
+	struct eth_dev		*dev = &l_ethdev;
 
-	struct eth_dev *dev = &l_ethdev;
-	dprintf("%s:...\n",__func__);
+	VDEBUG(dev, "%s:...\n", __func__);
 
 	req = dev->tx_req;
 
@@ -1836,7 +1839,7 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le
 	retval = usb_ep_queue (dev->in_ep, req, GFP_ATOMIC);
 
 	if (!retval)
-		dprintf("%s: packet queued\n",__func__);
+		VDEBUG(dev, "%s: packet queued\n",__func__);
 	while(!packet_sent)
 	{
 		packet_sent=0;
@@ -1853,7 +1856,7 @@ static int usb_eth_recv(struct eth_device* netdev)
 
 	if (packet_received)
 	{
-		dprintf("%s: packet received \n",__func__);
+		VDEBUG(dev, "%s: packet received \n",__func__);
 		if (dev->rx_req)
 		{
 			NetReceive(NetRxPackets[0],dev->rx_req->length);
@@ -1862,7 +1865,7 @@ static int usb_eth_recv(struct eth_device* netdev)
 			if (dev->rx_req)
 				rx_submit (dev, dev->rx_req, 0);
 		}
-		else printf("dev->rx_req invalid\n");
+		else WARN(dev, "dev->rx_req invalid\n");
 	}
 	return 0;
 }
@@ -1873,7 +1876,7 @@ void usb_eth_halt(struct eth_device* netdev)
 
 	if (!netdev)
 	{
-		printf("ERROR: received NULL ptr\n");
+		ERROR(dev, "ERROR: received NULL ptr\n");
 		return;
 	}
 
@@ -1929,11 +1932,11 @@ int usb_eth_initialize(bd_t *bi)
 	host_addr[sizeof(host_addr)-1] = '\0';
 
 	if (!is_eth_addr_valid(dev_addr)) {
-		printf("ERROR: Need valid 'usbnet_devaddr' to be set\n");
+		ERROR(dev, "ERROR: Need valid 'usbnet_devaddr' to be set\n");
 		status = -1;
 	}
 	if (!is_eth_addr_valid(host_addr)) {
-		printf("ERROR: Need valid 'usbnet_hostaddr' to be set\n");
+		ERROR(dev, "ERROR: Need valid 'usbnet_hostaddr' to be set\n");
 		status = -1;
 	}
 	if (status)
@@ -1947,7 +1950,7 @@ int usb_eth_initialize(bd_t *bi)
 	return 0;
 
 fail:
-	printf("%s failed\n", __func__ );
+	ERROR(dev, "%s failed. error = %d\n", __func__, status);
 	return status;
 }
 
-- 
1.7.1.1

[U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout

[Sergei Shtylyov]

Hello.

Vitaly Kuzmichev wrote:

> Take debug printout macros back from linux-2.6.27 and make them more
> useful and more compatible.

> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> ---
>  drivers/usb/gadget/ether.c |   65 +++++++++++++++++++++++---------------------
>  1 files changed, 34 insertions(+), 31 deletions(-)

> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
> index a07738f..b6f5f4d 100644
> --- a/drivers/usb/gadget/ether.c
> +++ b/drivers/usb/gadget/ether.c
[...]
> @@ -769,7 +771,7 @@ set_ether_config (struct eth_dev *dev, gfp_t gfp_flags)
>  
>  		result = usb_ep_enable (dev->status_ep, dev->status);
>  		if (result != 0) {
> -			printf ("enable %s --> %d\n",
> +			DEBUG (dev, "enable %s --> %d\n",
>  				dev->status_ep->name, result);
>  			goto done;
>  		}
> @@ -789,14 +791,14 @@ set_ether_config (struct eth_dev *dev, gfp_t gfp_flags)
>  	if (!cdc_active(dev)) {
>  		result = usb_ep_enable (dev->in_ep, dev->in);
>  		if (result != 0) {
> -			printf ("enable %s --> %d\n",
> +			DEBUG(dev, "enable %s --> %d\n",

   Well, I think the coding style shouldbe consistent -- you either leave the 
space before ( or remove it. And as U-Boot seems to follow the Linux coding 
style now, it seems better to remove the space.

> @@ -941,17 +945,17 @@ static void eth_status_complete (struct usb_ep *ep, struct usb_request *req)
>  
>  		req->length = STATUS_BYTECOUNT;
>  		value = usb_ep_queue (ep, req, GFP_ATOMIC);
> -		dprintf ("send SPEED_CHANGE --> %d\n", value);
> +		DEBUG (dev, "send SPEED_CHANGE --> %d\n", value);
>  		if (value == 0)
>  			return;
>  	} else if (value != -ECONNRESET) {
> -		dprintf("event %02x --> %d\n",
> +		DEBUG (dev, "event %02x --> %d\n",

    Why add a space where there was none before?

> @@ -1298,7 +1301,7 @@ fail:
>  
>  static void tx_complete (struct usb_ep *ep, struct usb_request *req)
>  {
> -	dprintf("%s, status: %s\n", __func__,(req->status) ? "failed":"ok");
> +	VDEBUG(ep->driver_data,

    No cast here?

> "%s: status %s\n", __func__, (req->status)?"failed":"ok");
>  	packet_sent=1;
>  }

WBR, Sergei

[U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout

[Vitaly Kuzmichev]

Hi Sergei,

Sergei Shtylyov wrote:
>> -            printf ("enable %s --> %d\n",
>> +            DEBUG(dev, "enable %s --> %d\n",
> 
>   Well, I think the coding style shouldbe consistent -- you either leave
> the space before ( or remove it. And as U-Boot seems to follow the Linux
> coding style now, it seems better to remove the space.

Well, I would say: "either leave the space before ( or remove it from
EVERYWHERE".
I think that the last thing should be done as a separate patch.
So I would like to keep the coding style that is in this file.


Best regards,
Vitaly.

[U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout

[Sergei Shtylyov]

Hello.

Vitaly Kuzmichev wrote:

>>> -            printf ("enable %s --> %d\n",
>>> +            DEBUG(dev, "enable %s --> %d\n",

>>   Well, I think the coding style shouldbe consistent -- you either leave
>> the space before ( or remove it. And as U-Boot seems to follow the Linux
>> coding style now, it seems better to remove the space.

> Well, I would say: "either leave the space before ( or remove it from
> EVERYWHERE".
> I think that the last thing should be done as a separate patch.
> So I would like to keep the coding style that is in this file.

    Which you've failed to do so far. ;-)

> Best regards,
> Vitaly.

WBR, Sergei

[U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout

[Remy Bohmer]

Hi,

2010/8/12 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
> Take debug printout macros back from linux-2.6.27 and make them more
> useful and more compatible.
>
> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> ---
> ?drivers/usb/gadget/ether.c | ? 65 +++++++++++++++++++++++---------------------
> ?1 files changed, 34 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
> index a07738f..b6f5f4d 100644
> --- a/drivers/usb/gadget/ether.c
> +++ b/drivers/usb/gadget/ether.c
> @@ -37,8 +37,10 @@
> ?#define dev_err(x, stuff...) printf(stuff)
> ?#define dev_dbg dev_err
> ?#define dev_warn dev_err
> -#define DEBUG dev_err
> -#define VDEBUG DEBUG
> +#define WARN INFO
> +#define ERROR INFO
> +#define DEBUG INFO

This switches DEBUG logging on by default. This is not wanted.
Can you please change that?

Kind regards,

Remy

[U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout

[Vitaly Kuzmichev]

Hi Remy,

No, it does not.
DEBUG is defined as dev_err and dev_err is defined as printf.
Anyway I can change it.

On 08/12/2010 10:33 PM, Remy Bohmer wrote:
>> @@ -37,8 +37,10 @@
>>  #define dev_err(x, stuff...) printf(stuff)
>>  #define dev_dbg dev_err
>>  #define dev_warn dev_err
>> -#define DEBUG dev_err
>> -#define VDEBUG DEBUG
>> +#define WARN INFO
>> +#define ERROR INFO
>> +#define DEBUG INFO
> 
> This switches DEBUG logging on by default. This is not wanted.
> Can you please change that?
> 
> Kind regards,
> 
> Remy


-- 
Best regards,
Vitaly Kuzmichev.

[U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout

[Remy Bohmer]

Hi,

2010/8/13 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
> Hi Remy,
>
> No, it does not.
> DEBUG is defined as dev_err and dev_err is defined as printf.
> Anyway I can change it.

Please do not top-post!

> On 08/12/2010 10:33 PM, Remy Bohmer wrote:
>>> @@ -37,8 +37,10 @@
>>> ?#define dev_err(x, stuff...) printf(stuff)
>>> ?#define dev_dbg dev_err
>>> ?#define dev_warn dev_err
>>> -#define DEBUG dev_err
>>> -#define VDEBUG DEBUG
>>> +#define WARN INFO
>>> +#define ERROR INFO
>>> +#define DEBUG INFO
>>
>> This switches DEBUG logging on by default. This is not wanted.
>> Can you please change that?

> No, it does not.

Well, I see with this patch much more debug logging then without it...
Hmm, it seems that you replaced all use of dprintf (which is trashed)
by DEBUG()...

> Anyway I can change it.

please do, DEBUG logging should not be on by default...

Kind regards,

Remy

[U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout

[Wolfgang Denk]

Dear Remy Bohmer,

In message <AANLkTi=gnFZr6wT-RJ6CLFEKuGGhhpJUDbj1VftAR9kX@mail.gmail.com> you wrote:
> 
> >>> +#define WARN INFO
> >>> +#define ERROR INFO
> >>> +#define DEBUG INFO
> >>
> >> This switches DEBUG logging on by default. This is not wanted.
> >> Can you please change that?
> 
> > No, it does not.
> 
> Well, I see with this patch much more debug logging then without it...
> Hmm, it seems that you replaced all use of dprintf (which is trashed)
> by DEBUG()...
> 
> > Anyway I can change it.
> 
> please do, DEBUG logging should not be on by default...

DEBUG is already a well-defined name. Any different use of the same
name will result in the patches being rejected.

Please fix this!

I also object against names like WARN, ERROR and INFO. They are just
too dangerous.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You are an excellent tactician, Captain. You let your second in  com-
mand attack while you sit and watch for weakness.
	-- Khan Noonian Singh, "Space Seed", stardate 3141.9

[U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout

[Remy Bohmer]

Hi,

> DEBUG is already a well-defined name. Any different use of the same
> name will result in the patches being rejected.
>
> Please fix this!
>
> I also object against names like WARN, ERROR and INFO. They are just
> too dangerous.

Agree.

Remy

[U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout

[Stefano Babic]

Vitaly Kuzmichev wrote:
> Take debug printout macros back from linux-2.6.27 and make them more
> useful and more compatible.
> 
> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> ---
>  drivers/usb/gadget/ether.c |   65 +++++++++++++++++++++++---------------------
>  1 files changed, 34 insertions(+), 31 deletions(-)
> 

Hi Vitaly,

a general comment. In u-boot is DEBUG generally defined to activate the
output, as you see in a lot of drivers. It should not be changed as
meanining in a single driver.

However, it seems more consistent to apply the comment sent by Reinhard:

http://lists.denx.de/pipermail/u-boot/2010-August/075346.html

debug() is already provided in u-boot, it makes sense not to add another
slightly modification of a debug output.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH 4/8] USB-CDC: Correct freeing usb requests

[Vitaly Kuzmichev]

Fix in_ep and out_ep confusion (rx_req was allocated from out_ep, not
from in_ep) and add lost dev->req freeing.

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/ether.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index b6f5f4d..dd3ab8c 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -845,7 +845,7 @@ static void eth_reset_config (struct eth_dev *dev)
 	if (dev->out) {
 		usb_ep_disable (dev->out_ep);
 		if (dev->rx_req) {
-			usb_ep_free_request (dev->in_ep, dev->rx_req);
+			usb_ep_free_request (dev->out_ep, dev->rx_req);
 			dev->rx_req=NULL;
 		}
 	}
@@ -1432,6 +1432,11 @@ static void eth_unbind (struct usb_gadget *gadget)
 
 	DEBUG (dev, "%s...\n", __func__);
 
+	/* we've already been disconnected ... no i/o is active */
+	if (dev->req) {
+		usb_ep_free_request (gadget->ep0, dev->req);
+		dev->req = NULL;
+	}
 	if (dev->stat_req) {
 		usb_ep_free_request (dev->status_ep, dev->stat_req);
 		dev->stat_req = NULL;
@@ -1443,7 +1448,7 @@ static void eth_unbind (struct usb_gadget *gadget)
 	}
 
 	if (dev->rx_req) {
-		usb_ep_free_request (dev->in_ep, dev->rx_req);
+		usb_ep_free_request (dev->out_ep, dev->rx_req);
 		dev->rx_req=NULL;
 	}
 
-- 
1.7.1.1

[U-Boot] [PATCH 5/8] USB-CDC: Replace 'strcpy' by 'strlcpy'

[Vitaly Kuzmichev]

Replace 'strcpy' by more safe 'strlcpy' that is implemented in ether.c

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/ether.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index dd3ab8c..25d6243 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -1600,12 +1600,12 @@ static int eth_bind(struct usb_gadget *gadget)
 	if (bcdDevice)
 		device_desc.bcdDevice = cpu_to_le16(bcdDevice);
 	if (iManufacturer)
-		strcpy (manufacturer, iManufacturer);
+		strlcpy (manufacturer, iManufacturer, sizeof manufacturer);
 	if (iProduct)
-		strcpy (product_desc, iProduct);
+		strlcpy (product_desc, iProduct, sizeof product_desc);
 	if (iSerialNumber) {
 		device_desc.iSerialNumber = STRING_SERIALNUMBER,
-		strcpy(serial_number, iSerialNumber);
+		strlcpy(serial_number, iSerialNumber, sizeof serial_number);
 	}
 
 	/* all we really need is bulk IN/OUT */
-- 
1.7.1.1

[U-Boot] [PATCH 6/8] USB-CDC: Correct stat_req initialization

[Vitaly Kuzmichev]

Fix possible oops on stat_req->buf initialization and fix ep0 and
status_ep confusion (last one is just intended for stat_req keeping).

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/ether.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 25d6243..5710ddf 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -1739,14 +1739,13 @@ autoconf_fail:
 	/* ... and maybe likewise for status transfer */
 #if defined(DEV_CONFIG_CDC)
 	if (dev->status_ep) {
-		dev->stat_req = usb_ep_alloc_request(gadget->ep0, GFP_KERNEL);
-		dev->stat_req->buf = status_req;
+		dev->stat_req = usb_ep_alloc_request(dev->status_ep, GFP_KERNEL);
 		if (!dev->stat_req) {
-			dev->stat_req->buf=NULL;
-			usb_ep_free_request (gadget->ep0, dev->req);
+			usb_ep_free_request (dev->status_ep, dev->req);
 
 			goto fail;
 		}
+		dev->stat_req->buf = status_req;
 		dev->stat_req->context = NULL;
 	}
 #endif
-- 
1.7.1.1

[U-Boot] [PATCH 7/8] USB: gadget: ethernet error path potential oops fix

[Vitaly Kuzmichev]

From: David Brownell <david-b@pacbell.net>

Fix potential (never-observed) oops on rare error path,
bugzilla #9594.  Fix uses the same test as used earlier.

Also make the adjacent "else" block look like an "else" block
instead of hiding like a bug.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

(cherry picked from commit e7b13ec9235b9fded90f826ceeb8c34548631351)

Conflicts:

	drivers/usb/gadget/ether.c
		Cause: "else" block was removed while porting.
		Removing this part of the patch.

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/ether.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 5710ddf..8f0f5be 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -810,7 +810,7 @@ done:
 
 	/* on error, disable any endpoints  */
 	if (result < 0) {
-		if (!subset_active(dev))
+		if (!subset_active(dev) && dev->status_ep)
 			(void) usb_ep_disable (dev->status_ep);
 		dev->status = NULL;
 		(void) usb_ep_disable (dev->in_ep);
-- 
1.7.1.1

[U-Boot] [PATCH 7/8] USB: gadget: ethernet error path potential oops fix

[Sergei Shtylyov]

Hello.

Vitaly Kuzmichev wrote:

> From: David Brownell <david-b@pacbell.net>

> Fix potential (never-observed) oops on rare error path,
> bugzilla #9594.  Fix uses the same test as used earlier.

    I think references to bugzilla.kernel.org bugs look out of place in the 
U-Boot patch's changelog.

> Also make the adjacent "else" block look like an "else" block
> instead of hiding like a bug.

> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

> (cherry picked from commit e7b13ec9235b9fded90f826ceeb8c34548631351)

    Hm, I'm not sure you can cherry-pick across different projects. :-)
You'd better describe this patch as *based* on that commit.

> Conflicts:

> 	drivers/usb/gadget/ether.c
> 		Cause: "else" block was removed while porting.
> 		Removing this part of the patch.

> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>

WBR, Sergei

[U-Boot] [PATCH 8/8] USB: gadget: change simple_strtol to simple_strtoul

[Vitaly Kuzmichev]

From: Julia Lawall <julia@diku.dk>

Since num is unsigned, it would seem better to use simple_strtoul that
simple_strtol.

A simplified version of the semantic patch that makes this change is as
follows: (http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@r2@
long e;
position p;
@@

e = simple_strtol at p(...)

@@
position p != r2.p;
type T;
T e;
@@

e =
- simple_strtol at p
+ simple_strtoul
  (...)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>
Acked-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

(cherry picked from commit bb9496c6f7e853e5d4edd5397c9d45f1968d623c)

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/epautoconf.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index c7fad39..e11cc20 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -156,7 +156,7 @@ ep_matches (
 
 	/* report address */
 	if (isdigit (ep->name [2])) {
-		u8	num = simple_strtol (&ep->name [2], NULL, 10);
+		u8	num = simple_strtoul (&ep->name [2], NULL, 10);
 		desc->bEndpointAddress |= num;
 #ifdef	MANY_ENDPOINTS
 	} else if (desc->bEndpointAddress & USB_DIR_IN) {
-- 
1.7.1.1

[U-Boot] [PATCH 0/8] USB: gadget: fix porting bugs

[Remy Bohmer]

Hi,

2010/8/12 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
> 1) Restuct Makefile to unable usage old and new stacks simultaneously
> 4) Fix in_ep, out_ep, ep0 and status_ep confusions
> 5) Replace 'strcpy' by 'strlcpy'
> 6) Fix possible oops on stat_req->buf initialization

OK

> 2) Add lost 'qmult' definition for High Speed devices

Stefano, which version of this fix do you prefer? It seems there are
now 2 patches fixing the same thing...

> 3) Take debug printout macros back and make them more useful
> 7) 2 updates backported from linux-2.6.git

Besides the already made review comments: OK

Please fix. Thanks.

Kind regards,

Remy

[U-Boot] [PATCH 0/8] USB: gadget: fix porting bugs

[stefano babic]

Il 12/08/2010 20:37, Remy Bohmer ha scritto:
> Hi,
> 

Hi,

> 2010/8/12 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
>> 1) Restuct Makefile to unable usage old and new stacks simultaneously
>> 4) Fix in_ep, out_ep, ep0 and status_ep confusions
>> 5) Replace 'strcpy' by 'strlcpy'
>> 6) Fix possible oops on stat_req->buf initialization
> 
> OK
> 
>> 2) Add lost 'qmult' definition for High Speed devices
> 
> Stefano, which version of this fix do you prefer? It seems there are
> now 2 patches fixing the same thing...

It is not a problem for me to get Vitaly's patch, the only important
thing is that the issue is fixed !
Vitaly makes additionally qmult configurable using a CONFIG_ switch, we
can get his patch.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH 0/8] USB: gadget: fix porting bugs

[Vitaly Kuzmichev]

Hi Remy,

On 08/12/2010 10:37 PM, Remy Bohmer wrote:
> Hi,
> 
>> 2) Add lost 'qmult' definition for High Speed devices
> 
> Stefano, which version of this fix do you prefer? It seems there are
> now 2 patches fixing the same thing...
> 

If I were you I would take my patch for 2 reasons:
1) more accurate comment (this is not OTG-related)
2) it is configurable as it was in the kernel

As alternative way I can add Stefano's signature in the patch. I like
this idea :)
What do you think, Stefano?

[U-Boot] [PATCH 0/8] USB: gadget: fix porting bugs

[Stefano Babic]

Vitaly Kuzmichev wrote:
> If I were you I would take my patch for 2 reasons:
> 1) more accurate comment (this is not OTG-related)
> 2) it is configurable as it was in the kernel
> 
> As alternative way I can add Stefano's signature in the patch. I like
> this idea :)
> What do you think, Stefano?
> 

Hi Vitaly,

I have already agreed about the patch. Your patch makes qmult
configurable, and there is no side-effects to replace mine.

Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Vitaly Kuzmichev]

Replace Linux-like debug printout macros by native ones.

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/ether.c |   99 ++++++++++++++++++++-----------------------
 1 files changed, 46 insertions(+), 53 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index a07738f..c287ec2 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -20,6 +20,9 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#define DEBUG
+#undef DEBUG
+
 #include <common.h>
 #include <asm/errno.h>
 #include <linux/usb/ch9.h>
@@ -31,14 +34,7 @@
 #include "gadget_chips.h"
 
 #define USB_NET_NAME "usb0"
-#define dprintf(x, ...)
-#undef INFO
-#define INFO(x, s...)	printf(s)
-#define dev_err(x, stuff...) printf(stuff)
-#define dev_dbg dev_err
-#define dev_warn dev_err
-#define DEBUG dev_err
-#define VDEBUG DEBUG
+
 #define atomic_read
 extern struct platform_data brd;
 #define spin_lock(x)
@@ -769,7 +765,7 @@ set_ether_config (struct eth_dev *dev, gfp_t gfp_flags)
 
 		result = usb_ep_enable (dev->status_ep, dev->status);
 		if (result != 0) {
-			printf ("enable %s --> %d\n",
+			debug("enable %s --> %d\n",
 				dev->status_ep->name, result);
 			goto done;
 		}
@@ -789,14 +785,14 @@ set_ether_config (struct eth_dev *dev, gfp_t gfp_flags)
 	if (!cdc_active(dev)) {
 		result = usb_ep_enable (dev->in_ep, dev->in);
 		if (result != 0) {
-			printf ("enable %s --> %d\n",
+			debug("enable %s --> %d\n",
 				dev->in_ep->name, result);
 			goto done;
 		}
 
 		result = usb_ep_enable (dev->out_ep, dev->out);
 		if (result != 0) {
-			printf ("enable %s --> %d\n",
+			debug("enable %s --> %d\n",
 				dev->out_ep->name, result);
 			goto done;
 		}
@@ -827,6 +823,8 @@ static void eth_reset_config (struct eth_dev *dev)
 	if (dev->config == 0)
 		return;
 
+	debug("%s\n", __func__);
+
 	/* disable endpoints, forcing (synchronous) completion of
 	 * pending i/o.  then free the requests.
 	 */
@@ -864,7 +862,7 @@ static int eth_set_config (struct eth_dev *dev, unsigned number, gfp_t gfp_flags
 			&& dev->config
 			&& dev->tx_qlen != 0) {
 		/* tx fifo is full, but we can't clear it...*/
-		INFO (dev, "can't change configurations\n");
+		error("can't change configurations");
 		return -ESPIPE;
 	}
 	eth_reset_config (dev);
@@ -901,7 +899,7 @@ static int eth_set_config (struct eth_dev *dev, unsigned number, gfp_t gfp_flags
 		}
 
 		dev->config = number;
-		INFO (dev, "%s speed config #%d: %d mA, %s, using %s\n",
+		printf("%s speed config #%d: %d mA, %s, using %s\n",
 				speed, number, power, driver_desc,
 				(cdc_active(dev)? "CDC Ethernet"
 						: "CDC Ethernet Subset"));
@@ -941,11 +939,11 @@ static void eth_status_complete (struct usb_ep *ep, struct usb_request *req)
 
 		req->length = STATUS_BYTECOUNT;
 		value = usb_ep_queue (ep, req, GFP_ATOMIC);
-		dprintf ("send SPEED_CHANGE --> %d\n", value);
+		debug("send SPEED_CHANGE --> %d\n", value);
 		if (value == 0)
 			return;
 	} else if (value != -ECONNRESET) {
-		dprintf("event %02x --> %d\n",
+		debug("event %02x --> %d\n",
 			event->bNotificationType, value);
 		if (event->bNotificationType==
 				USB_CDC_NOTIFY_SPEED_CHANGE)
@@ -991,7 +989,7 @@ static void issue_start_status (struct eth_dev *dev)
 
 	value = usb_ep_queue (dev->status_ep, req, GFP_ATOMIC);
 	if (value < 0)
-		printf ("status buf queue --> %d\n", value);
+		debug("status buf queue --> %d\n", value);
 }
 
 #endif
@@ -1001,8 +999,7 @@ static void issue_start_status (struct eth_dev *dev)
 static void eth_setup_complete (struct usb_ep *ep, struct usb_request *req)
 {
 	if (req->status || req->actual != req->length)
-		dprintf  (/*(struct eth_dev *) ep->driver_data*/
-				"setup complete --> %d, %d/%d\n",
+		debug("setup complete --> %d, %d/%d\n",
 				req->status, req->actual, req->length);
 }
 
@@ -1029,7 +1026,7 @@ eth_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 	 * while config change events may enable network traffic.
 	 */
 
-	dprintf("eth_setup:...\n");
+	debug("%s\n", __func__);
 
 	req->complete = eth_setup_complete;
 	switch (ctrl->bRequest) {
@@ -1078,9 +1075,9 @@ eth_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 		if (ctrl->bRequestType != 0)
 			break;
 		if (gadget->a_hnp_support)
-			DEBUG (dev, "HNP available\n");
+			debug("HNP available\n");
 		else if (gadget->a_alt_hnp_support)
-			DEBUG (dev, "HNP needs a different root port\n");
+			debug("HNP needs a different root port\n");
 		value = eth_set_config (dev, wValue, GFP_ATOMIC);
 		break;
 	case USB_REQ_GET_CONFIGURATION:
@@ -1145,7 +1142,7 @@ eth_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 		/* FIXME this is wrong, as is the assumption that
 		 * all non-PXA hardware talks real CDC ...
 		 */
-		dev_warn (&gadget->dev, "set_interface ignored!\n");
+		debug("set_interface ignored!\n");
 #endif /* DEV_CONFIG_CDC */
 
 done_set_intf:
@@ -1179,7 +1176,7 @@ done_set_intf:
 				|| wLength != 0
 				|| wIndex > 1)
 			break;
-		printf ("packet filter %02x\n", wValue);
+		debug("packet filter %02x\n", wValue);
 		dev->cdc_filter = wValue;
 		value = 0;
 		break;
@@ -1194,21 +1191,20 @@ done_set_intf:
 #endif /* DEV_CONFIG_CDC */
 
 	default:
-		printf (
-			"unknown control req%02x.%02x v%04x i%04x l%d\n",
+		debug("unknown control req%02x.%02x v%04x i%04x l%d\n",
 			ctrl->bRequestType, ctrl->bRequest,
 			wValue, wIndex, wLength);
 	}
 
 	/* respond with data transfer before status phase? */
 	if (value >= 0) {
-		dprintf("respond with data transfer before status phase\n");
+		debug("respond with data transfer before status phase\n");
 		req->length = value;
 		req->zero = value < wLength
 				&& (value % gadget->ep0->maxpacket) == 0;
 		value = usb_ep_queue (gadget->ep0, req, GFP_ATOMIC);
 		if (value < 0) {
-			DEBUG (dev, "ep_queue --> %d\n", value);
+			debug("ep_queue --> %d\n", value);
 			req->status = 0;
 			eth_setup_complete (gadget->ep0, req);
 		}
@@ -1237,7 +1233,7 @@ static int rx_submit ( struct eth_dev *dev, struct usb_request *req, \
 	 * byte off the end (to force hardware errors on overflow).
 	 */
 
-	dprintf("%s\n", __func__);
+	debug("%s\n", __func__);
 
 	size = (ETHER_HDR_SIZE + dev->mtu + RX_EXTRA);
 	size += dev->out_ep->maxpacket - 1;
@@ -1255,7 +1251,7 @@ static int rx_submit ( struct eth_dev *dev, struct usb_request *req, \
 	retval = usb_ep_queue (dev->out_ep, req, gfp_flags);
 
 	if (retval) {
-		dprintf ("rx submit --> %d\n", retval);
+		error("rx submit --> %d", retval);
 	}
 	return retval;
 }
@@ -1265,8 +1261,7 @@ static void rx_complete (struct usb_ep *ep, struct usb_request *req)
 {
 	struct eth_dev	*dev = ep->driver_data;
 
-	dprintf("%s\n", __func__);
-	dprintf("rx status %d\n", req->status);
+	debug("%s: status %d\n", __func__, req->status);
 
 	packet_received=1;
 
@@ -1291,14 +1286,14 @@ static int alloc_requests (struct eth_dev *dev, unsigned n, gfp_t gfp_flags)
 	return 0;
 
 fail:
-	DEBUG (dev, "can't alloc requests\n");
+	error("can't alloc requests");
 	return -1;
 }
 
 
 static void tx_complete (struct usb_ep *ep, struct usb_request *req)
 {
-	dprintf("%s, status: %s\n", __func__,(req->status) ? "failed":"ok");
+	debug("%s: status %s\n", __func__, (req->status)?"failed":"ok");
 	packet_sent=1;
 }
 
@@ -1427,7 +1422,7 @@ static void eth_unbind (struct usb_gadget *gadget)
 {
 	struct eth_dev *dev = get_gadget_data (gadget);
 
-	printf("eth_unbind:...\n");
+	debug("%s...\n", __func__);
 
 	if (dev->stat_req) {
 		usb_ep_free_request (dev->status_ep, dev->stat_req);
@@ -1567,8 +1562,7 @@ static int eth_bind(struct usb_gadget *gadget)
 		 * anything less functional on CDC-capable hardware,
 		 * so we fail in this case.
 		 */
-		dev_err (&gadget->dev,
-			"controller '%s' not recognized\n",
+		error("controller '%s' not recognized",
 			gadget->name);
 		return -ENODEV;
 	}
@@ -1605,8 +1599,7 @@ static int eth_bind(struct usb_gadget *gadget)
 	in_ep = usb_ep_autoconfig (gadget, &fs_source_desc);
 	if (!in_ep) {
 autoconf_fail:
-		dev_err (&gadget->dev,
-			"can't autoconfigure on %s\n",
+		error("can't autoconfigure on %s\n",
 			gadget->name);
 		return -ENODEV;
 	}
@@ -1700,18 +1693,18 @@ autoconf_fail:
 			dev->host_mac [2], dev->host_mac [3],
 			dev->host_mac [4], dev->host_mac [5]);
 
-	INFO (dev, "using %s, OUT %s IN %s%s%s\n", gadget->name,
+	printf("using %s, OUT %s IN %s%s%s\n", gadget->name,
 		out_ep->name, in_ep->name,
 		status_ep ? " STATUS " : "",
 		status_ep ? status_ep->name : ""
 		);
-	INFO (dev, "MAC %02x:%02x:%02x:%02x:%02x:%02x\n",
+	printf("MAC %02x:%02x:%02x:%02x:%02x:%02x\n",
 		dev->net->enetaddr [0], dev->net->enetaddr [1],
 		dev->net->enetaddr [2], dev->net->enetaddr [3],
 		dev->net->enetaddr [4], dev->net->enetaddr [5]);
 
 	if (cdc) {
-		INFO (dev, "HOST MAC %02x:%02x:%02x:%02x:%02x:%02x\n",
+		printf("HOST MAC %02x:%02x:%02x:%02x:%02x:%02x\n",
 			dev->host_mac [0], dev->host_mac [1],
 			dev->host_mac [2], dev->host_mac [3],
 			dev->host_mac [4], dev->host_mac [5]);
@@ -1755,7 +1748,7 @@ autoconf_fail:
 	return 0;
 
 fail:
-	dev_dbg(&gadget->dev, "register_netdev failed\n");
+	error("%s failed", __func__);
 	eth_unbind (gadget);
 	return -ENOMEM;
 }
@@ -1768,7 +1761,7 @@ static int usb_eth_init(struct eth_device* netdev, bd_t* bd)
 	unsigned long timeout = USB_CONNECT_TIMEOUT;
 
 	if (!netdev) {
-		printf("ERROR: received NULL ptr\n");
+		error("received NULL ptr");
 		goto fail;
 	}
 
@@ -1790,7 +1783,7 @@ static int usb_eth_init(struct eth_device* netdev, bd_t* bd)
 	{
 		/* Handle control-c and timeouts */
 		if (ctrlc() || (get_timer(ts) > timeout)) {
-			printf("The remote end did not respond in time.\n");
+			error("The remote end did not respond in time.");
 			goto fail;
 		}
 		usb_gadget_handle_interrupts();
@@ -1806,9 +1799,9 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le
 {
 	int			retval;
 	struct usb_request	*req = NULL;
+	struct eth_dev		*dev = &l_ethdev;
 
-	struct eth_dev *dev = &l_ethdev;
-	dprintf("%s:...\n",__func__);
+	debug("%s:...\n", __func__);
 
 	req = dev->tx_req;
 
@@ -1836,7 +1829,7 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le
 	retval = usb_ep_queue (dev->in_ep, req, GFP_ATOMIC);
 
 	if (!retval)
-		dprintf("%s: packet queued\n",__func__);
+		debug("%s: packet queued\n", __func__);
 	while(!packet_sent)
 	{
 		packet_sent=0;
@@ -1853,7 +1846,7 @@ static int usb_eth_recv(struct eth_device* netdev)
 
 	if (packet_received)
 	{
-		dprintf("%s: packet received \n",__func__);
+		debug("%s: packet received \n", __func__);
 		if (dev->rx_req)
 		{
 			NetReceive(NetRxPackets[0],dev->rx_req->length);
@@ -1862,7 +1855,7 @@ static int usb_eth_recv(struct eth_device* netdev)
 			if (dev->rx_req)
 				rx_submit (dev, dev->rx_req, 0);
 		}
-		else printf("dev->rx_req invalid\n");
+		else error("dev->rx_req invalid");
 	}
 	return 0;
 }
@@ -1873,7 +1866,7 @@ void usb_eth_halt(struct eth_device* netdev)
 
 	if (!netdev)
 	{
-		printf("ERROR: received NULL ptr\n");
+		error("received NULL ptr");
 		return;
 	}
 
@@ -1929,11 +1922,11 @@ int usb_eth_initialize(bd_t *bi)
 	host_addr[sizeof(host_addr)-1] = '\0';
 
 	if (!is_eth_addr_valid(dev_addr)) {
-		printf("ERROR: Need valid 'usbnet_devaddr' to be set\n");
+		error("Need valid 'usbnet_devaddr' to be set");
 		status = -1;
 	}
 	if (!is_eth_addr_valid(host_addr)) {
-		printf("ERROR: Need valid 'usbnet_hostaddr' to be set\n");
+		error("Need valid 'usbnet_hostaddr' to be set");
 		status = -1;
 	}
 	if (status)
@@ -1947,7 +1940,7 @@ int usb_eth_initialize(bd_t *bi)
 	return 0;
 
 fail:
-	printf("%s failed\n", __func__ );
+	error("%s failed. error = %d", __func__, status);
 	return status;
 }
 
-- 
1.7.1.1

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Rogan Dawes]

On 2010/08/13 2:57 PM, Vitaly Kuzmichev wrote:
> Replace Linux-like debug printout macros by native ones.
>
> Signed-off-by: Vitaly Kuzmichev<vkuzmichev@mvista.com>
> ---
>   drivers/usb/gadget/ether.c |   99 ++++++++++++++++++++-----------------------
>   1 files changed, 46 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
> index a07738f..c287ec2 100644
> --- a/drivers/usb/gadget/ether.c
> +++ b/drivers/usb/gadget/ether.c
> @@ -20,6 +20,9 @@
>    * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>    */
>
> +#define DEBUG
> +#undef DEBUG
> +

Eh?

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Vitaly Kuzmichev]

Hi,

Rogan Dawes wrote:
>> +#define DEBUG
>> +#undef DEBUG
>> +
> 
> Eh?

Such thing is used to let someone know that this driver supports debug
output through native U-Boot macros. So one need to comment #undef to
compile ether.c with debug messages.
There are at least 67 files in U-Boot that use such construction.

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Vitaly Kuzmichev]

Vitaly Kuzmichev wrote:
> There are at least 67 files in U-Boot that use such construction.
I was wrong.
I calculated for "linux/drivers" directory.
Actually 13 for u-boot.

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Stefano Babic]

Vitaly Kuzmichev wrote:
> Hi,
> 
> Rogan Dawes wrote:
>>> +#define DEBUG
>>> +#undef DEBUG
>>> +
>> Eh?
> 
> Such thing is used to let someone know that this driver supports debug
> output through native U-Boot macros. So one need to comment #undef to
> compile ether.c with debug messages.
> There are at least 67 files in U-Boot that use such construction.

Well, but probably it is better to remove both lines. In the rest of
u-boot, DEBUG is neither set or unset - you see only #ifdef DEBUG or
#ifndef DEBUG. You have found the examples how we should not do...

If you want to remember how to set the debug output, it should be enough
to add a comments with "to enable the debugging, define DEBUG before
common.h" or something like that. I vote to remove only the two lines...

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Vitaly Kuzmichev]

Hi Stefano,

Stefano Babic wrote:
> If you want to remember how to set the debug output, it should be enough
> to add a comments with "to enable the debugging, define DEBUG before
> common.h" or something like that. I vote to remove only the two lines...

Would not it be better to make one of the following?
1)
#if 0
#define DEBUG
#endif

2)
#ifdef CONFIG_USB_ETH_DEBUG
#define DEBUG
#endif

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Rogan Dawes]

On 2010/08/13 3:40 PM, Vitaly Kuzmichev wrote:
> Hi Stefano,
>
> Stefano Babic wrote:
>> If you want to remember how to set the debug output, it should be enough
>> to add a comments with "to enable the debugging, define DEBUG before
>> common.h" or something like that. I vote to remove only the two lines...
>
> Would not it be better to make one of the following?
> 1)
> #if 0
> #define DEBUG
> #endif
>
> 2)
> #ifdef CONFIG_USB_ETH_DEBUG
> #define DEBUG
> #endif

Personally, I like option 2, as it means that I have less need to touch 
general code when trying to debug my board.

I can just define that in my board config. Although, that could lead to 
a proliferation of *_DEBUG defines, which may or may not be documented.

Perhaps without the CONFIG_ part, and people who are trying to debug 
their boards need to read the code anyway to determine what debugging 
can be enabled?

Rogan

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Wolfgang Denk]

Dear Vitaly Kuzmichev,

In message <4C654B30.9060106@mvista.com> you wrote:
> Hi Stefano,
> 
> Stefano Babic wrote:
> > If you want to remember how to set the debug output, it should be enough
> > to add a comments with "to enable the debugging, define DEBUG before
> > common.h" or something like that. I vote to remove only the two lines...
> 
> Would not it be better to make one of the following?
> 1)
> #if 0
> #define DEBUG
> #endif
> 
> 2)
> #ifdef CONFIG_USB_ETH_DEBUG
> #define DEBUG
> #endif

NAK.

It is so trivial o #define DEBUG (either in the file or on the command
line) that thhere is zero need for any "clever" wrappers around this.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Behind every great man, there is a woman -- urging him on.
	-- Harry Mudd, "I, Mudd", stardate 4513.3

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Stefano Babic]

Vitaly Kuzmichev wrote:
> Hi Stefano,
>

Hi Vitaly,

> Stefano Babic wrote:
>> If you want to remember how to set the debug output, it should be enough
>> to add a comments with "to enable the debugging, define DEBUG before
>> common.h" or something like that. I vote to remove only the two lines...
> 
> Would not it be better to make one of the following?
> 1)
> #if 0
> #define DEBUG
> #endif

However, this is seen as dead code. As this one, my preferance is to set
only a comment (I see several files in u-boot with such a comment, for
example arch/arm/lib/board.c).

> 
> 2)
> #ifdef CONFIG_USB_ETH_DEBUG
> #define DEBUG
> #endif

It is ok, but it generates another new CONFIG_ switch, that is unusable
for the rest of u-boot. I agree that in u-boot there is a lot of
different and local functions to setup, and probably a general mechanism
should be better defined. However, setting "#define DEBUG" before any
include files is quite usual. I do not like to set a CONFIG_ switch only
for debug purpose, as in the delivered system all debug output should be
turned off.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Wolfgang Denk]

Dear Stefano Babic,

In message <4C6557C5.4000300@denx.de> you wrote:
>
> It is ok, but it generates another new CONFIG_ switch, that is unusable
> for the rest of u-boot. I agree that in u-boot there is a lot of
> different and local functions to setup, and probably a general mechanism
> should be better defined. However, setting "#define DEBUG" before any
> include files is quite usual. I do not like to set a CONFIG_ switch only
> for debug purpose, as in the delivered system all debug output should be
> turned off.

We already have such a general mechanism, the problem is just that
people ignore it and re-invent the wheel.

We have debug(), debugX(), error(), and BUG_ON().

What exactly seems to be missing?

Note that when using constants as "level" argument in debugX() the
compiler can even optimize away non-relevant code.

I remember we had a similar discussion in the past, which resulted
that I accepted to have debugX() added - yet how many files use it?
2 (in words: TWO)!

So before we add even more debug utilities someone has to bring up
really good arguments to convince me.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Where shall I begin, please your Majesty?" he asked. "Begin  at  the
beginning,"  the  King said, gravely, "and go on till you come to the
end: then stop."    - Alice's Adventures in Wonderland, Lewis Carroll

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Wolfgang Denk]

Dear Vitaly Kuzmichev,

In message <4C6544B9.3000503@mvista.com> you wrote:
> 
> >> +#define DEBUG
> >> +#undef DEBUG
> >> +
> > 
> > Eh?
> 
> Such thing is used to let someone know that this driver supports debug
> output through native U-Boot macros. So one need to comment #undef to
> compile ether.c with debug messages.
> There are at least 67 files in U-Boot that use such construction.

Patches to clean up these are welcome.

I will NAK this whenever I see it.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
PLEASE NOTE: Some Quantum Physics Theories Suggest That When the Con-
sumer Is Not Directly Observing This Product, It May Cease  to  Exist
or Will Exist Only in a Vague and Undetermined State.

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Sergei Shtylyov]

Hello.

Vitaly Kuzmichev wrote:

> Rogan Dawes wrote:

>>> +#define DEBUG
>>> +#undef DEBUG
>>> +

>> Eh?

> Such thing is used to let someone know that this driver supports debug
> output through native U-Boot macros. So one need to comment #undef to
> compile ether.c with debug messages.
> There are at least 67 files in U-Boot that use such construction.

    I'd suggest leaving just #undef DEBUG (which can be edited into #define 
DEBUG if desired)...

WBR, Sergei

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Wolfgang Denk]

Dear Sergei Shtylyov,

In message <4C6558D4.9070308@mvista.com> you wrote:
> 
>     I'd suggest leaving just #undef DEBUG (which can be edited into #define 
> DEBUG if desired)...

Don't you listen?  Remove that, completely, please!

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Commitment, n.:      Commitment can be illustrated by a breakfast
of ham and eggs. The chicken was involved, the pig was committed.

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Wolfgang Denk]

Dear Vitaly Kuzmichev,

In message <1281704276-29115-1-git-send-email-vkuzmichev@mvista.com> you wrote:
> Replace Linux-like debug printout macros by native ones.
> 
> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> ---
>  drivers/usb/gadget/ether.c |   99 ++++++++++++++++++++-----------------------
>  1 files changed, 46 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
> index a07738f..c287ec2 100644
> --- a/drivers/usb/gadget/ether.c
> +++ b/drivers/usb/gadget/ether.c
> @@ -20,6 +20,9 @@
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>   */
>  
> +#define DEBUG
> +#undef DEBUG
> +

NAK. Please remove this dead code.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Every program has at least one bug and can be shortened by  at  least
one  instruction  --  from  which,  by induction, one can deduce that
every program can be reduced to one instruction which doesn't work.

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Remy Bohmer]

Hi,

2010/8/13 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
> Replace Linux-like debug printout macros by native ones.
>
> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> ---
> ?drivers/usb/gadget/ether.c | ? 99 ++++++++++++++++++++-----------------------
> ?1 files changed, 46 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
> index a07738f..c287ec2 100644
> --- a/drivers/usb/gadget/ether.c
> +++ b/drivers/usb/gadget/ether.c
> @@ -20,6 +20,9 @@
> ?* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA ?02111-1307 ?USA
> ?*/
>
> +#define DEBUG
> +#undef DEBUG

Applied to u-boot-usb/cdc branch after removing ' #define DEBUG' as
Sergei suggested.
Thanks.

Remy

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Wolfgang Denk]

Dear Remy Bohmer,

In message <AANLkTikBeeSeKKuAECPZbWM3RAmJkjcpKk=ykrNHwVPW@mail.gmail.com> you wrote:
> 
> > +#define DEBUG
> > +#undef DEBUG
> 
> Applied to u-boot-usb/cdc branch after removing ' #define DEBUG' as
> Sergei suggested.

NAK. Please also remove the dead and completely needless #undef DEBUG
as well.

Thanks.


Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If the odds are a million to one against something occuring,  chances
are 50-50 it will.

[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros

[Remy Bohmer]

Hi,

2010/8/13 Wolfgang Denk <wd@denx.de>:
> Dear Remy Bohmer,
>
> In message <AANLkTikBeeSeKKuAECPZbWM3RAmJkjcpKk=ykrNHwVPW@mail.gmail.com> you wrote:
>>
>> > +#define DEBUG
>> > +#undef DEBUG
>>
>> Applied to u-boot-usb/cdc branch after removing ' #define DEBUG' as
>> Sergei suggested.
>
> NAK. Please also remove the dead and completely needless #undef DEBUG
> as well.

Done!

Remy

[U-Boot] [PATCH v2 4/8] USB-CDC: Correct freeing usb requests

[Vitaly Kuzmichev]

Fix in_ep and out_ep confusion (rx_req was allocated from out_ep, not
from in_ep) and add lost dev->req freeing.

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/ether.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index c287ec2..19a63de 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -839,7 +839,7 @@ static void eth_reset_config (struct eth_dev *dev)
 	if (dev->out) {
 		usb_ep_disable (dev->out_ep);
 		if (dev->rx_req) {
-			usb_ep_free_request (dev->in_ep, dev->rx_req);
+			usb_ep_free_request (dev->out_ep, dev->rx_req);
 			dev->rx_req=NULL;
 		}
 	}
@@ -1424,6 +1424,11 @@ static void eth_unbind (struct usb_gadget *gadget)
 
 	debug("%s...\n", __func__);
 
+	/* we've already been disconnected ... no i/o is active */
+	if (dev->req) {
+		usb_ep_free_request (gadget->ep0, dev->req);
+		dev->req = NULL;
+	}
 	if (dev->stat_req) {
 		usb_ep_free_request (dev->status_ep, dev->stat_req);
 		dev->stat_req = NULL;
@@ -1435,7 +1440,7 @@ static void eth_unbind (struct usb_gadget *gadget)
 	}
 
 	if (dev->rx_req) {
-		usb_ep_free_request (dev->in_ep, dev->rx_req);
+		usb_ep_free_request (dev->out_ep, dev->rx_req);
 		dev->rx_req=NULL;
 	}
 
-- 
1.7.1.1

[U-Boot] [PATCH v2 4/8] USB-CDC: Correct freeing usb requests

[Remy Bohmer]

Hi,

2010/8/13 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
> Fix in_ep and out_ep confusion (rx_req was allocated from out_ep, not
> from in_ep) and add lost dev->req freeing.
>
> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> ---
> ?drivers/usb/gadget/ether.c | ? ?9 +++++++--
> ?1 files changed, 7 insertions(+), 2 deletions(-)

Applied to u-boot-usb/cdc branch.
Thanks.

Remy

[U-Boot] [PATCH v2 5/8] USB-CDC: Replace 'strcpy' by 'strlcpy'

[Vitaly Kuzmichev]

Replace 'strcpy' by more safe 'strlcpy' that is implemented in ether.c

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/ether.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 19a63de..65f3ff9 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -1591,12 +1591,12 @@ static int eth_bind(struct usb_gadget *gadget)
 	if (bcdDevice)
 		device_desc.bcdDevice = cpu_to_le16(bcdDevice);
 	if (iManufacturer)
-		strcpy (manufacturer, iManufacturer);
+		strlcpy (manufacturer, iManufacturer, sizeof manufacturer);
 	if (iProduct)
-		strcpy (product_desc, iProduct);
+		strlcpy (product_desc, iProduct, sizeof product_desc);
 	if (iSerialNumber) {
 		device_desc.iSerialNumber = STRING_SERIALNUMBER,
-		strcpy(serial_number, iSerialNumber);
+		strlcpy(serial_number, iSerialNumber, sizeof serial_number);
 	}
 
 	/* all we really need is bulk IN/OUT */
-- 
1.7.1.1

[U-Boot] [PATCH v2 5/8] USB-CDC: Replace 'strcpy' by 'strlcpy'

[Remy Bohmer]

Hi,

2010/8/13 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
> Replace 'strcpy' by more safe 'strlcpy' that is implemented in ether.c
>
> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> ---
> ?drivers/usb/gadget/ether.c | ? ?6 +++---
> ?1 files changed, 3 insertions(+), 3 deletions(-)
>

Applied to u-boot-usb/cdc branch.
Thanks.

Remy

[U-Boot] [PATCH v2 6/8] USB-CDC: Correct stat_req initialization

[Vitaly Kuzmichev]

Fix possible oops on stat_req->buf initialization and fix ep0 and
status_ep confusion (last one is just intended for stat_req keeping).

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 drivers/usb/gadget/ether.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 65f3ff9..03d8f0b 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -1729,14 +1729,13 @@ autoconf_fail:
 	/* ... and maybe likewise for status transfer */
 #if defined(DEV_CONFIG_CDC)
 	if (dev->status_ep) {
-		dev->stat_req = usb_ep_alloc_request(gadget->ep0, GFP_KERNEL);
-		dev->stat_req->buf = status_req;
+		dev->stat_req = usb_ep_alloc_request(dev->status_ep, GFP_KERNEL);
 		if (!dev->stat_req) {
-			dev->stat_req->buf=NULL;
-			usb_ep_free_request (gadget->ep0, dev->req);
+			usb_ep_free_request (dev->status_ep, dev->req);
 
 			goto fail;
 		}
+		dev->stat_req->buf = status_req;
 		dev->stat_req->context = NULL;
 	}
 #endif
-- 
1.7.1.1

[U-Boot] [PATCH v2 6/8] USB-CDC: Correct stat_req initialization

[Remy Bohmer]

Hi,

2010/8/13 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
> Fix possible oops on stat_req->buf initialization and fix ep0 and
> status_ep confusion (last one is just intended for stat_req keeping).
>
> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> ---
> ?drivers/usb/gadget/ether.c | ? ?7 +++----
> ?1 files changed, 3 insertions(+), 4 deletions(-)
>

Applied to u-boot-usb/cdc branch.
Thanks.

Remy

[U-Boot] [PATCH v2 7/8] USB-CDC: ethernet error path potential oops fix

[Vitaly Kuzmichev]

Fix potential oops on rare error path.
The patch is based on commit e7b13ec9235b9fded90f826ceeb8c34548631351
(done by David Brownell <david-b@pacbell.net>) from linux-2.6.git.

Description of the issue taken from linux kernel bugzilla:
(https://bugzilla.kernel.org/show_bug.cgi?id=9594)

The potential error can be tracked down as follows:

(1) line 807: let the second conjunct on the "if" statment be false
    meaning "dev->status_ep" is null. This means the "if" evaluates
    to false.

follow thru the code until...

(2) line 808: usb_ep_disable(dev->status_ep) passes in a null argument,
    however "usb_ep_disable" cannot handle that:

(from include/linux/usb/gadget.h)
191 static inline int
192 usb_ep_disable (struct usb_ep *ep)
193 {
194         return ep->ops->disable (ep);
195 }

--

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/ether.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 03d8f0b..62dd008 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -804,7 +804,7 @@ done:
 
 	/* on error, disable any endpoints  */
 	if (result < 0) {
-		if (!subset_active(dev))
+		if (!subset_active(dev) && dev->status_ep)
 			(void) usb_ep_disable (dev->status_ep);
 		dev->status = NULL;
 		(void) usb_ep_disable (dev->in_ep);
-- 
1.7.1.1

[U-Boot] [PATCH v2 7/8] USB-CDC: ethernet error path potential oops fix

[Remy Bohmer]

Hi,

2010/8/13 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
> Fix potential oops on rare error path.
> The patch is based on commit e7b13ec9235b9fded90f826ceeb8c34548631351
> (done by David Brownell <david-b@pacbell.net>) from linux-2.6.git.
>
> Description of the issue taken from linux kernel bugzilla:
> (https://bugzilla.kernel.org/show_bug.cgi?id=9594)
>
> The potential error can be tracked down as follows:
>
> (1) line 807: let the second conjunct on the "if" statment be false
> ? ?meaning "dev->status_ep" is null. This means the "if" evaluates
> ? ?to false.
>
> follow thru the code until...
>
> (2) line 808: usb_ep_disable(dev->status_ep) passes in a null argument,
> ? ?however "usb_ep_disable" cannot handle that:
>
> (from include/linux/usb/gadget.h)
> 191 static inline int
> 192 usb_ep_disable (struct usb_ep *ep)
> 193 {
> 194 ? ? ? ? return ep->ops->disable (ep);
> 195 }
>
> --
>
> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> ---
> ?drivers/usb/gadget/ether.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>

Applied to u-boot-usb/cdc branch.
Thanks.

Remy

[U-Boot] [PATCH v2 8/8] USB-CDC: change simple_strtol to simple_strtoul

[Vitaly Kuzmichev]

The patch is based on commit bb9496c6f7e853e5d4edd5397c9d45f1968d623c
(done by Julia Lawall <julia@diku.dk>) from linux-2.6.git.

Since num is unsigned, it would seem better to use simple_strtoul that
simple_strtol.

Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
---
 drivers/usb/gadget/epautoconf.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index c7fad39..e11cc20 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -156,7 +156,7 @@ ep_matches (
 
 	/* report address */
 	if (isdigit (ep->name [2])) {
-		u8	num = simple_strtol (&ep->name [2], NULL, 10);
+		u8	num = simple_strtoul (&ep->name [2], NULL, 10);
 		desc->bEndpointAddress |= num;
 #ifdef	MANY_ENDPOINTS
 	} else if (desc->bEndpointAddress & USB_DIR_IN) {
-- 
1.7.1.1

[U-Boot] [PATCH v2 8/8] USB-CDC: change simple_strtol to simple_strtoul

[Remy Bohmer]

Hi,

2010/8/13 Vitaly Kuzmichev <vkuzmichev@mvista.com>:
> The patch is based on commit bb9496c6f7e853e5d4edd5397c9d45f1968d623c
> (done by Julia Lawall <julia@diku.dk>) from linux-2.6.git.
>
> Since num is unsigned, it would seem better to use simple_strtoul that
> simple_strtol.
>
> Signed-off-by: Vitaly Kuzmichev <vkuzmichev@mvista.com>
> ---
> ?drivers/usb/gadget/epautoconf.c | ? ?2 +-

Applied to u-boot-usb/cdc branch.
Thanks.

Remy