Re: [U-Boot] [PATCH v2 1/5] usb:gadget:s5p USB Device Controller (UDC) implementation
Hi Lukasz, On 07/13/2011 06:29 PM, Lukasz Majewski wrote: This commit provides UDC driver support for Samsung's SoC family of processors. [...] diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index 7d5b504..6717e4f 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -26,9 +26,9 @@ include $(TOPDIR)/config.mk LIB := $(obj)libusb_gadget.o # new USB gadget layer dependencies -ifdef CONFIG_USB_ETHER -COBJS-y += ether.o epautoconf.o config.o usbstring.o -COBJS-$(CONFIG_USB_ETH_RNDIS) += rndis.o +ifdef CONFIG_USB_GADGET +COBJS-y += epautoconf.o config.o usbstring.o +COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o else # Devices not related to the new gadget layer depend on CONFIG_USB_DEVICE ifdef CONFIG_USB_DEVICE You're completely removing support of Ethernet and RNDIS gadgets. This is unacceptable. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v1 (WIP) 12/16] [Timer]Replace get_timer() usage in drivers/
Hello Graeme, Graeme Russ wrote: [...] diff --git a/drivers/fpga/spartan3.c b/drivers/fpga/spartan3.c index 1dd6f26..8282a23 100644 --- a/drivers/fpga/spartan3.c +++ b/drivers/fpga/spartan3.c [...] @@ -233,7 +235,7 @@ static int Spartan3_sp_load (Xilinx_desc * desc, void *buf, size_t bsize) #endif /* now check for done signal */ - ts = get_timer (0); /* get current time */ + ts = time_since_ms(); /* get current time */ Shouldn't time_now_ms() be here? Regards, Vitaly. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 0/5] USB RNDIS gadget support
The patch series integrates RNDIS protocol support into the current U-Boot USB gadget stack to talk with Windows host. Synced with linux-2.6.26 version (latest one that has simple, non-composite gadget architecture), therefore I've kept checkpatch warnings about typedefs and 80 character lines, but I'm glad to discuss necessity and ways to fix them. Actually this protocol should not require to make any changes in existing USB device controller drivers. And sorry for the huge RNDIS patch. Vitaly Kuzmichev (5): USB-CDC: handle interrupts after dropped pullup USB-CDC: Port struct net_device_stats USB-CDC: Move struct declaration before its use USB: Add USB RNDIS gadget protocol USB-RNDIS: Send RNDIS state on disconnecting drivers/usb/gadget/Makefile |1 + drivers/usb/gadget/ether.c | 777 ++--- drivers/usb/gadget/ndis.h | 217 +++ drivers/usb/gadget/rndis.c | 1317 +++ drivers/usb/gadget/rndis.h | 260 + include/linux/netdevice.h | 65 +++ include/linux/usb/cdc.h |6 +- 7 files changed, 2550 insertions(+), 93 deletions(-) create mode 100644 drivers/usb/gadget/ndis.h create mode 100644 drivers/usb/gadget/rndis.c create mode 100644 drivers/usb/gadget/rndis.h create mode 100644 include/linux/netdevice.h -- 1.7.3.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/5] USB-CDC: handle interrupt after dropped pullup
Disconnecting USB gadget with pending interrupt may cause its wrong
handling in the next time when interface will be started again
(especially actual for RNDIS). This interrupt may force the gadget
to queue unexpected response before setup stage.
Despite the fact that such interrupt handled after dropped pullup
also may add pending response, this will not bring to any issues due to
usb_ep_disable (which clears the queue) called on gadget unregistering.
Signed-off-by: Vitaly Kuzmichev vkuzmic...@mvista.com
---
drivers/usb/gadget/ether.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 01deeb1..079acea 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -1926,6 +1926,13 @@ void usb_eth_halt(struct eth_device *netdev)
return;
usb_gadget_disconnect(dev-gadget);
+
+ /* Clear pending interrupt */
+ if (dev-network_started) {
+ usb_gadget_handle_interrupts();
+ dev-network_started = 0;
+ }
+
usb_gadget_unregister_driver(eth_driver);
}
--
1.7.3.4
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/5] USB-CDC: Port struct net_device_stats
Port struct net_device_stats and statistics collecting needed for
RNDIS protocol.
Signed-off-by: Vitaly Kuzmichev vkuzmic...@mvista.com
---
drivers/usb/gadget/ether.c | 41 +++
include/linux/netdevice.h | 65
2 files changed, 106 insertions(+), 0 deletions(-)
create mode 100644 include/linux/netdevice.h
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 079acea..8aa6240 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -22,6 +22,7 @@
#include common.h
#include asm/errno.h
+#include linux/netdevice.h
#include linux/usb/ch9.h
#include linux/usb/cdc.h
#include linux/usb/gadget.h
@@ -175,6 +176,7 @@ struct eth_dev {
struct usb_request *tx_req, *rx_req;
struct eth_device *net;
+ struct net_device_stats stats;
unsigned inttx_qlen;
unsignedzlp:1;
@@ -1271,7 +1273,31 @@ static int rx_submit(struct eth_dev *dev, struct
usb_request *req,
static void rx_complete(struct usb_ep *ep, struct usb_request *req)
{
+ struct eth_dev *dev = ep-driver_data;
+
debug(%s: status %d\n, __func__, req-status);
+ switch (req-status) {
+ /* normal completion */
+ case 0:
+ dev-stats.rx_packets++;
+ dev-stats.rx_bytes += req-length;
+ break;
+
+ /* software-driven interface shutdown */
+ case -ECONNRESET: /* unlink */
+ case -ESHUTDOWN:/* disconnect etc */
+ /* for hardware automagic (such as pxa) */
+ case -ECONNABORTED: /* endpoint reset */
+ break;
+
+ /* data overrun */
+ case -EOVERFLOW:
+ dev-stats.rx_over_errors++;
+ /* FALLTHROUGH */
+ default:
+ dev-stats.rx_errors++;
+ break;
+ }
packet_received = 1;
}
@@ -1300,7 +1326,22 @@ fail1:
static void tx_complete(struct usb_ep *ep, struct usb_request *req)
{
+ struct eth_dev *dev = ep-driver_data;
+
debug(%s: status %s\n, __func__, (req-status) ? failed : ok);
+ switch (req-status) {
+ default:
+ dev-stats.tx_errors++;
+ debug(tx err %d\n, req-status);
+ /* FALLTHROUGH */
+ case -ECONNRESET: /* unlink */
+ case -ESHUTDOWN:/* disconnect etc */
+ break;
+ case 0:
+ dev-stats.tx_bytes += req-length;
+ }
+ dev-stats.tx_packets++;
+
packet_sent = 1;
}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
new file mode 100644
index 000..870d8b4
--- /dev/null
+++ b/include/linux/netdevice.h
@@ -0,0 +1,65 @@
+/*
+ * INETAn implementation of the TCP/IP protocol suite for the
LINUX
+ * operating system. INET is implemented using the BSD Socket
+ * interface as the means of communication with the user level.
+ *
+ * Definitions for the Interfaces handler.
+ *
+ * Version:@(#)dev.h 1.0.10 08/12/93
+ *
+ * Authors:Ross Biro
+ * Fred N. van Kempen, wal...@uwalt.nl.mugnet.org
+ * Corey Minyard wf-rch!miny...@relay.eu.net
+ * Donald J. Becker, bec...@cesdis.gsfc.nasa.gov
+ * Alan Cox, alan@linux.org
+ * Bjorn Ekwall. bj...@blox.se
+ * Pekka Riikonen priik...@poseidon.pspt.fi
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Moved to /usr/include/linux for NET3
+ */
+#ifndef _LINUX_NETDEVICE_H
+#define _LINUX_NETDEVICE_H
+
+/*
+ * Network device statistics. Akin to the 2.0 ether stats but
+ * with byte counters.
+ */
+
+struct net_device_stats {
+ unsigned long rx_packets; /* total packets received
*/
+ unsigned long tx_packets; /* total packets transmitted
*/
+ unsigned long rx_bytes; /* total bytes received
*/
+ unsigned long tx_bytes; /* total bytes transmitted
*/
+ unsigned long rx_errors; /* bad packets received
*/
+ unsigned long tx_errors; /* packet transmit problems
*/
+ unsigned long rx_dropped; /* no space in linux buffers
*/
+ unsigned long tx_dropped; /* no space available in linux
*/
+ unsigned long multicast; /* multicast packets received
*/
+ unsigned long collisions;
+
+ /* detailed rx_errors: */
+ unsigned long rx_length_errors;
+ unsigned long
[U-Boot] [PATCH 3/5] USB-CDC: Move struct declaration before its use
Signed-off-by: Vitaly Kuzmichev vkuzmic...@mvista.com
---
drivers/usb/gadget/ether.c | 70 ++-
1 files changed, 36 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 8aa6240..c070f63 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -104,6 +104,42 @@ static const char driver_desc[] = DRIVER_DESC;
#define USB_CONNECT_TIMEOUT (3 * CONFIG_SYS_HZ)
/*-*/
+
+struct eth_dev {
+ struct usb_gadget *gadget;
+ struct usb_request *req; /* for control responses */
+ struct usb_request *stat_req; /* for cdc status */
+
+ u8 config;
+ struct usb_ep *in_ep, *out_ep, *status_ep;
+ const struct usb_endpoint_descriptor
+ *in, *out, *status;
+
+ struct usb_request *tx_req, *rx_req;
+
+ struct eth_device *net;
+ struct net_device_stats stats;
+ unsigned inttx_qlen;
+
+ unsignedzlp:1;
+ unsignedcdc:1;
+ unsignedsuspended:1;
+ unsignednetwork_started:1;
+ u16 cdc_filter;
+ unsigned long todo;
+ int mtu;
+#defineWORK_RX_MEMORY 0
+ u8 host_mac[ETH_ALEN];
+};
+
+/*
+ * This version autoconfigures as much as possible at run-time.
+ *
+ * It also ASSUMES a self-powered device, without remote wakeup,
+ * although remote wakeup support would make sense.
+ */
+
+/*-*/
static struct eth_dev l_ethdev;
static struct eth_device l_netdev;
static struct usb_gadget_driver eth_driver;
@@ -163,40 +199,6 @@ static inline int BITRATE(struct usb_gadget *g)
}
#endif
-struct eth_dev {
- struct usb_gadget *gadget;
- struct usb_request *req; /* for control responses */
- struct usb_request *stat_req; /* for cdc status */
-
- u8 config;
- struct usb_ep *in_ep, *out_ep, *status_ep;
- const struct usb_endpoint_descriptor
- *in, *out, *status;
-
- struct usb_request *tx_req, *rx_req;
-
- struct eth_device *net;
- struct net_device_stats stats;
- unsigned inttx_qlen;
-
- unsignedzlp:1;
- unsignedcdc:1;
- unsignedsuspended:1;
- unsignednetwork_started:1;
- u16 cdc_filter;
- unsigned long todo;
- int mtu;
-#defineWORK_RX_MEMORY 0
- u8 host_mac[ETH_ALEN];
-};
-
-/*
- * This version autoconfigures as much as possible at run-time.
- *
- * It also ASSUMES a self-powered device, without remote wakeup,
- * although remote wakeup support would make sense.
- */
-
/*-*/
/*
--
1.7.3.4
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 5/5] USB-RNDIS: Send RNDIS state on disconnecting
Add waiting for receiving Ethernet gadget state on the Windows host
side before dropping pullup, but keep it for debug.
Signed-off-by: Vitaly Kuzmichev vkuzmic...@mvista.com
---
drivers/usb/gadget/ether.c | 23 +++
drivers/usb/gadget/rndis.c |5 +
drivers/usb/gadget/rndis.h |8
3 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index f909267..9fb0e80 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -1921,10 +1921,22 @@ static void eth_start(struct eth_dev *dev, gfp_t
gfp_flags)
static int eth_stop(struct eth_dev *dev)
{
+#ifdef RNDIS_COMPLETE_SIGNAL_DISCONNECT
+ unsigned long ts;
+ unsigned long timeout = CONFIG_SYS_HZ; /* 1 sec to stop RNDIS */
+#endif
+
if (rndis_active(dev)) {
rndis_set_param_medium(dev-rndis_config, NDIS_MEDIUM_802_3, 0);
rndis_signal_disconnect(dev-rndis_config);
+#ifdef RNDIS_COMPLETE_SIGNAL_DISCONNECT
+ /* Wait until host receives OID_GEN_MEDIA_CONNECT_STATUS */
+ ts = get_timer(0);
+ while (get_timer(ts) timeout)
+ usb_gadget_handle_interrupts();
+#endif
+
rndis_uninit(dev-rndis_config);
dev-rndis = 0;
}
@@ -2486,6 +2498,17 @@ void usb_eth_halt(struct eth_device *netdev)
if (!dev-gadget)
return;
+ /*
+* Some USB controllers may need additional deinitialization here
+* before dropping pull-up (also due to hardware issues).
+* For example: unhandled interrupt with status stage started may
+* bring the controller to fully broken state (until board reset).
+* There are some variants to debug and fix such cases:
+* 1) In the case of RNDIS connection eth_stop can perform additional
+* interrupt handling. See RNDIS_COMPLETE_SIGNAL_DISCONNECT definition.
+* 2) 'pullup' callback in your UDC driver can be improved to perform
+* this deinitialization.
+*/
eth_stop(dev);
usb_gadget_disconnect(dev-gadget);
diff --git a/drivers/usb/gadget/rndis.c b/drivers/usb/gadget/rndis.c
index 3e3f740..886c093 100644
--- a/drivers/usb/gadget/rndis.c
+++ b/drivers/usb/gadget/rndis.c
@@ -995,7 +995,12 @@ int rndis_signal_disconnect(int configNr)
rndis_per_dev_params[configNr].media_state
= NDIS_MEDIA_STATE_DISCONNECTED;
+#ifdef RNDIS_COMPLETE_SIGNAL_DISCONNECT
+ return rndis_indicate_status_msg(configNr,
+ RNDIS_STATUS_MEDIA_DISCONNECT);
+#else
return 0;
+#endif
}
void rndis_uninit(int configNr)
diff --git a/drivers/usb/gadget/rndis.h b/drivers/usb/gadget/rndis.h
index 73a1204..d9e3a75 100644
--- a/drivers/usb/gadget/rndis.h
+++ b/drivers/usb/gadget/rndis.h
@@ -17,6 +17,14 @@
#include ndis.h
+/*
+ * By default rndis_signal_disconnect does not send status message about
+ * RNDIS disconnection to USB host (indicated as cable disconnected).
+ * Define RNDIS_COMPLETE_SIGNAL_DISCONNECT to send it.
+ * However, this will cause 1 sec delay on Ethernet device halt.
+ * Usually you do not need to define it. Mostly usable for debugging.
+ */
+
#define RNDIS_MAXIMUM_FRAME_SIZE 1518
#define RNDIS_MAX_TOTAL_SIZE 1558
--
1.7.3.4
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] BeagleBoard-xM: Ethernet over USB supported ?
Hi Olivier, Olivier Martin wrote: Hi, I am trying to enable boot from tftp for a BeagleBoard-xM. The BeagleBoard-xM has an integrated Ethernet port over USB. I have tried to enable it by adding these following lines in include/configs/omap3_beagle.h: #define CONFIG_USB_ETHER 1 #define CONFIG_CMD_NET /* bootp, tftpboot, rarpboot*/ #define CONFIG_CMD_PING But when I compiled I have got a link error: /drivers/usb/gadget/libusb_gadget.o: In function `usb_eth_initialize': u-boot-main/drivers/usb/gadget/ether.c:1964: undefined reference to `usb_gadget_register_driver' drivers/usb/gadget/libusb_gadget.o: In function `usb_eth_init': u-boot-main/drivers/usb/gadget/ether.c:1812: undefined reference to `usb_gadget_handle_interrupts' The functions `usb_gadget_register_driver' and `usb_gadget_handle_interrupts' do not exist in the current u-boot tree... Of course. You haven't enabled appropriate USB gadget controller driver. Please learn the difference between USB Networking using USB host and USB gadget stacks. CONFIG_USB_ETHER is a gadget driver. You probably need CONFIG_USB_USBNET instead. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] usage of DMA
Hi Marcel,
Marcel wrote:
Could you enable debug output for ether.c and for your UDC driver and
show the results?
I enabled a lot of extra debugging messages so this is not very short.
The output attached below is including start of macb but USB device behave
the
same when I don't enable that.
[...]
USB network up!
ep3 status = 1040c40
rx_submit
size 1 = 1570
size 2 = 2081
size 3 = 2048
ep2: queue req 77fec8f0, len 2048
udc: invalid request
NO REQUEST BUF
Received ETH pack : 00 00 00 00 00 00 00 00
ERROR: rx submit -- -22
at ether.c:1289/rx_submit()
### main_loop entered: bootdelay=3
### main_loop: bootcmd=mtdparts default; nand read 0x7100 nand0,0; bootm
0x7100
Hit any key to stop autoboot: 0
Sam9g45
First, I don't completely understand how do you use the gadget driver.
You do not run any commands from u-boot prompt and your bootcmd does not
have any network actions.
Why your usb gadget driver becomes started?
Second, could you add the following debug printout into rx_submit and
check that NetRxPackets is really initialized?
@@ -1261,6 +1261,8 @@ static int rx_submit(struct eth_dev *dev
req-length = size;
req-complete = rx_complete;
+ printf(NetRxPackets[0] = %p !!!\n, NetRxPackets[0]);
+
retval = usb_ep_queue(dev-out_ep, req, gfp_flags);
if (retval)
---
Since NetRxPackets is defined as an array of pointers, it needs to be
initialized. And NetLoop does this:
volatile uchar *NetRxPackets[PKTBUFSRX];
...
int
NetLoop(proto_t protocol)
{
bd_t *bd = gd-bd;
...
if (!NetTxPacket) {
int i;
/*
* Setup packet buffers, aligned correctly.
*/
NetTxPacket = PktBuf[0] + (PKTALIGN - 1);
NetTxPacket -= (ulong)NetTxPacket % PKTALIGN;
for (i = 0; i PKTBUFSRX; i++) {
NetRxPackets[i] = NetTxPacket + (i+1)*PKTSIZE_ALIGN;
}
}
...
So I fear that your gadget is started manually. You should not do that!
The only thing that you can do with your udc driver during the board
initialization is the following:
static struct usba_platform_data usba_pdata = {
...
};
int board_eth_init(bd_t *bis)
{
int rc = 0;
#if defined(CONFIG_USB_ETHER) defined(CONFIG_USB_GADGET_ATMEL_USBA)
rc = usba_udc_probe(usba_pdata);
if (!rc)
rc = usb_eth_initialize(bis);
#endif
return rc;
}
If you want to autoboot from tftp you should specify bootcmd environment
variable to something like tftp uImage; bootm.
And always use usb gadget only with network commands like tftp, dhcp,
ping, etc.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] usage of DMA
Hi Marcel, Marcel wrote: I have already reduced the code quite far. Basically all dma related stuff is gone. According to your first message in this thread I suppose that you are using atmel_usba_udc driver. Am I right? Looks like it's able to use PIO mode transfer (I mean linux driver). Each EP has can_dma flag which can be passed through platform_data. So: 1) Could you please make sure that this driver is working in linux in PIO mode (with can_dma set to zero)? 2) Could you port this driver to u-boot w/o changes in functions that talk with hardware registers and try to use PIO mode (with can_dma set to zero and commented DMA function calls)? The device is recognised by the host without errors, but there are some issues left as the OUT data doesn't get processed by ether.c. After the device descriptor is received the host usually wants to setup the device. Has the setup stage been done? I'm pulling my hair on this one, but since the data arrives in the OUT fifo and I can read it from the controller part I guess all end points are really up and running but the interfacing to the gadget system maybe isn't correct. Perhaps that's related to the driver I'm porting which comes from a 2.6.33 kernel. 2.6.33 controller driver should be compatible with 2.6.24 gadget stack (which was ported to u-boot). I read quite a lot of the USB device specification for my controller and so far think I've done everything right regarding the registers. Nobody can be sure in such thing :) Even if everything works Could you enable debug output for ether.c and for your UDC driver and show the results? With best regards, Vitaly. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] usage of DMA
Hi Marcel, In addition, did you enable CONFIG_USB_GADGET_DUALSPEED option in your u-boot config file? With best regards, Vitaly. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 0/2] USB-CDC: Minor usability fixes
Here are two patches to make USB Ethernet gadget driver more usable. Vitaly Kuzmichev (2): USB-CDC: Do not rename netdev after its registration USB-CDC: Move MAC addresses setting into usb_eth_init drivers/usb/gadget/ether.c | 67 ++- 1 files changed, 28 insertions(+), 39 deletions(-) -- 1.7.2.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/2] USB-CDC: Do not rename netdev after its registration
Calling eth_bind at usb_eth_init time causes renaming of the network device from 'usb_ether' to 'usb0'. Fixing this to keep the first name. Signed-off-by: Vitaly Kuzmichev vkuzmic...@mvista.com --- drivers/usb/gadget/ether.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 261cf7e..765fbd8 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -30,7 +30,7 @@ #include gadget_chips.h -#define USB_NET_NAME usb0 +#define USB_NET_NAME usb_ether #define atomic_read extern struct platform_data brd; @@ -1687,7 +1687,6 @@ autoconf_fail: } dev-net = l_netdev; - strcpy(dev-net-name, USB_NET_NAME); dev-cdc = cdc; dev-zlp = zlp; @@ -1924,7 +1923,7 @@ int usb_eth_initialize(bd_t *bi) int status = 0; struct eth_device *netdev = l_netdev; - sprintf(netdev-name, usb_ether); + strlcpy(netdev-name, USB_NET_NAME, sizeof(netdev-name)); netdev-init = usb_eth_init; netdev-send = usb_eth_send; -- 1.7.2.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/2] USB-CDC: Move MAC addresses setting into usb_eth_init
This allows to change device and host MAC addresses without performing
reset.
Signed-off-by: Vitaly Kuzmichev vkuzmic...@mvista.com
---
drivers/usb/gadget/ether.c | 62 ++-
1 files changed, 26 insertions(+), 36 deletions(-)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 765fbd8..6384869 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -1788,6 +1788,32 @@ static int usb_eth_init(struct eth_device *netdev, bd_t
*bd)
error(received NULL ptr);
goto fail;
}
+
+ /* Configure default mac-addresses for the USB ethernet device */
+#ifdef CONFIG_USBNET_DEV_ADDR
+ strlcpy(dev_addr, CONFIG_USBNET_DEV_ADDR, sizeof(dev_addr));
+#endif
+#ifdef CONFIG_USBNET_HOST_ADDR
+ strlcpy(host_addr, CONFIG_USBNET_HOST_ADDR, sizeof(host_addr));
+#endif
+ /* Check if the user overruled the MAC addresses */
+ if (getenv(usbnet_devaddr))
+ strlcpy(dev_addr, getenv(usbnet_devaddr),
+ sizeof(dev_addr));
+
+ if (getenv(usbnet_hostaddr))
+ strlcpy(host_addr, getenv(usbnet_hostaddr),
+ sizeof(host_addr));
+
+ if (!is_eth_addr_valid(dev_addr)) {
+ error(Need valid 'usbnet_devaddr' to be set);
+ goto fail;
+ }
+ if (!is_eth_addr_valid(host_addr)) {
+ error(Need valid 'usbnet_hostaddr' to be set);
+ goto fail;
+ }
+
if (usb_gadget_register_driver(eth_driver) 0)
goto fail;
@@ -1920,7 +1946,6 @@ static struct usb_gadget_driver eth_driver = {
int usb_eth_initialize(bd_t *bi)
{
- int status = 0;
struct eth_device *netdev = l_netdev;
strlcpy(netdev-name, USB_NET_NAME, sizeof(netdev-name));
@@ -1933,41 +1958,6 @@ int usb_eth_initialize(bd_t *bi)
#ifdef CONFIG_MCAST_TFTP
#error not supported
#endif
- /* Configure default mac-addresses for the USB ethernet device */
-#ifdef CONFIG_USBNET_DEV_ADDR
- strncpy(dev_addr, CONFIG_USBNET_DEV_ADDR, sizeof(dev_addr));
-#endif
-#ifdef CONFIG_USBNET_HOST_ADDR
- strncpy(host_addr, CONFIG_USBNET_HOST_ADDR, sizeof(host_addr));
-#endif
- /* Check if the user overruled the MAC addresses */
- if (getenv(usbnet_devaddr))
- strncpy(dev_addr, getenv(usbnet_devaddr),
- sizeof(dev_addr));
-
- if (getenv(usbnet_hostaddr))
- strncpy(host_addr, getenv(usbnet_hostaddr),
- sizeof(host_addr));
-
- /* Make sure both strings are terminated */
- dev_addr[sizeof(dev_addr)-1] = '\0';
- host_addr[sizeof(host_addr)-1] = '\0';
-
- if (!is_eth_addr_valid(dev_addr)) {
- error(Need valid 'usbnet_devaddr' to be set);
- status = -1;
- }
- if (!is_eth_addr_valid(host_addr)) {
- error(Need valid 'usbnet_hostaddr' to be set);
- status = -1;
- }
- if (status)
- goto fail;
-
eth_register(netdev);
return 0;
-
-fail:
- error(%s failed. error = %d, __func__, status);
- return status;
}
--
1.7.2.3
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] usb_ether: register usb ethernet gadget at each eth init
Hi Lei, Lei Wen wrote: Since the ether may not be the only one usb gadget would be used in the uboot, it is neccessary to do the register each time the eth begin to work to make usb gadget driver less confussed when we want to use two different usb gadget at the same time. Usb gadget driver could simple ignore the register operation, if it find the driver has been registered already. Signed-off-by: Lei Wen lei...@marvell.com Tested, works fine on my board. BTW, for future reference the tags in the subject should be [U-Boot] [PATCH v2]. v2 points to that this is an improved/fixed version of a patch previously submitted. This can be reached using --subject-prefix=U-Boot] [PATCH v2 passed to git format-patch command. Best regards, Vitaly. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] usb_ether: register usb ethernet gadget at each eth init
Hi Lei, Lei Wen wrote: Hi Vitaly, [...] And additional calling of usb_gadget_unregister_driver will at most return an error. For current ether.c state, there is no usb_gadget_unregister_driver in it. Even it has, we still need usb_gadget_register_driver call in each eth_init(). Yes, if we do 'unregister' we should do 'register' somewhere before. If we do 'register' we should do 'unregister' somewhere after. This is the symmetry such like in dynamic data allocation (like malloc/free), and we should comply it. The reason why there is no 'usb_gadget_unregister_driver' in the Ether driver yet is that there is no symmetrical function for 'usb_eth_initialize' because there is no way to remove a network device from the U-Boot environment. [...] I think the gadget driver should adopt like this to allow two gadget coexist. And from gadget driver's review, the switch from one gadget to another is easy, just register it to another should be enough. Does it really need to return EBUSY? If the UDC (not a gadget) driver does not support more than 1 gadget driver in the same time it must return an error (EBUSY is fine for now) when another gadget tries to register itself. Right, if at the same time certainly need to return error. So your code that expects successful register will make Ether gadget broken... [...] I really do not understand what a problem with using 2 gadgets in the same time without your patch if the UDC driver supports this. Especially I do not understand... Sorry, maybe I make a confused saying here. I don't mean to support two gadget at the same time, but to let one gadget could work after another gadget is finished. Like after tftp, I could use fastboot, or after return from fastboot, I could still use the tftp. ...But if you add usb_gadget_unregister_driver in eth_halt (and additional checking that dev-gadget is not equal to NULL) all existing UDC drivers (mine and Remy's at91_udc) will be working fine. And your fastboot will be working too. [...] ...how can the gadget (or UDC?) driver be confused here? [...] The confused thing is for the ether.c would register its gadget at the usb_eth_initialize() call, which is a very begining of the uboot. If we use some other gadget, like fastboot, it would take place of gadget registered in the gadget driver. You probably meant in the UDC driver? Note that USB gadget stack consists of 2 sorts of drivers: gadget and controller (UDC, USB device controller). The gadget driver provides hardware independent protocol for talking with host machine. The controller driver interacts with hardware - USB device chip. The UDC driver allocates 'struct gadget' (passed in 'usb_gadget_driver.bind' callback) for each gadget driver which tries to register itself. However, all existing UDC drivers (both in Linux and U-Boot) are not able to handle more than 1 gadget driver in the same time. And now I guess that this impossible. So they all return EBUSY if another gadget tries to register itself. This means that if you want 2 gadgets you need to register each one right before transferring data and unregister right after the data was transferred. By doing gadget unregister you will free allocated resources (such as USB endpoints and USB requests) in UDC and Ether drivers properly. Otherwise you will have memory leaks. Best regards, Vitaly. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] usb_ether: register usb ethernet gadget at each eth init
Hi Lei, Lei Wen wrote: [...] For current ether.c state, there is no usb_gadget_unregister_driver in it. Even it has, we still need usb_gadget_register_driver call in each eth_init(). Yes, if we do 'unregister' we should do 'register' somewhere before. If we do 'register' we should do 'unregister' somewhere after. This is the symmetry such like in dynamic data allocation (like malloc/free), and we should comply it. The reason why there is no 'usb_gadget_unregister_driver' in the Ether driver yet is that there is no symmetrical function for 'usb_eth_initialize' because there is no way to remove a network device from the U-Boot environment. What we need to do is not trying to remove the network device, but to register another gadget when we want to switch from tftp to fastboot, and make UDC take a different protocol to communicate with host. I have not asked you to remove network device, I have talked about the reason for the following: For current ether.c state, there is no usb_gadget_unregister_driver in it. [...] Right, if at the same time certainly need to return error. So your code that expects successful register will make Ether gadget broken... Don't understand here, why the Ether gadget would be broken? Because UDC driver does not allow to register more than 1 gadget driver in the same time. It just returns EBUSY. Even if the first gadget driver is idle (when Ether gadget driver is registered but tftp command is not issued). This is done to protect against memory leaks. Just checkout cdc-at91 branch in u-boot-usb.git for example of UDC driver (drivers/usb/gadget/at91_udc.c). Or look at any UDC driver in linux kernel. [...] This means that if you want 2 gadgets you need to register each one right before transferring data and unregister right after the data was transferred. By doing gadget unregister you will free allocated resources (such as USB endpoints and USB requests) in UDC and Ether drivers properly. Otherwise you will have memory leaks. Sure, so we comes into a conclusion that add register call in the eth_init and unregister call in eth_halt? In unregister call, the UDC driver should free the ep as you said. Yes, it will free by 'eth_unbind' called from 'usb_gadget_unregister_driver'. Note that 'usb_gadget_register_driver' should be removed from 'usb_eth_initialize' and dev-gadget checking should be added in eth_halt. Best regards, Lei Best regards, Vitaly. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] usb_ether: register usb ethernet gadget at each eth init
Hi Lei, Lei Wen wrote: Hi Vitaly, [...] + if (usb_gadget_register_driver(eth_driver) 0) + goto fail; You probably need then at least to remove usb_gadget_register_driver call from usb_eth_initialize. And add usb_gadget_unregister_driver in usb_eth_halt. I am afraid that remove usb_gadget_register_driver call from usb_eth_initialize is not possible. For tftp as example, it would call eth_halt before the eth_init. If we do the remove behavior, the gadget still not valid yet in the first eth_halt call which would cause the uboot becomes halt. The gadget driver handles this properly. Right now it just does usb_gadget_disconnect which has no effect when no gadget was requested from the UDC driver yet. And additional calling of usb_gadget_unregister_driver will at most return an error. For example, my UDC driver returns EBUSY if another gadget driver tries to register itself. (Yes, I do not want to have more than 1 gadget). With your patch the gadget driver will fail each time when initializing. Anyway if I fix my UDC driver some day the Ether gadget will try to register itself twice (and more times later). So I will have to protect it against this bad behavior. Two gadget example may comes from what I am using now. I am current attempting to make the tftp and fastboot cowork togetther. Since fastboot is a cool tool, I think people may want it beside they still use the tftp. Would that be enough convincing?:) I think the gadget driver should adopt like this to allow two gadget coexist. And from gadget driver's review, the switch from one gadget to another is easy, just register it to another should be enough. Does it really need to return EBUSY? If the UDC (not a gadget) driver does not support more than 1 gadget driver in the same time it must return an error (EBUSY is fine for now) when another gadget tries to register itself. The reason why does not it support 2 (and more) gadgets registered simultaneously is another question. Even if I fix this, your patch is not needed because the UDC driver will give USB requests to the appropriate gadget driver properly. I really do not understand what a problem with using 2 gadgets in the same time without your patch if the UDC driver supports this. Especially I do not understand... it is neccessary to do the register each time the eth begin to work to make usb gadget driver less confussed when we want to use two different usb gadget at the same time. ...how can the gadget (or UDC?) driver be confused here? Right now I can tell that it will be more confused rather than less. I just can suppose that you are trying to implement something like switching active gadget driver in UDC driver which has only one variable to store info about gadget driver (rather than having an array at least). In this case you need to fix UDC driver rather than hacking all available gadget drivers. Also this way will not work when someone needs two gadgets active simultaneously, e.g. USB ether with USB serial. Best regards, Vitaly. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] usb_ether: register usb ethernet gadget at each eth init
Vitaly Kuzmichev wrote: Hi Lei, Lei Wen wrote: Hi Vitaly, [...] + if (usb_gadget_register_driver(eth_driver) 0) + goto fail; You probably need then at least to remove usb_gadget_register_driver call from usb_eth_initialize. And add usb_gadget_unregister_driver in usb_eth_halt. I am afraid that remove usb_gadget_register_driver call from usb_eth_initialize is not possible. For tftp as example, it would call eth_halt before the eth_init. If we do the remove behavior, the gadget still not valid yet in the first eth_halt call which would cause the uboot becomes halt. The gadget driver handles this properly. Right now it just does usb_gadget_disconnect which has no effect when no gadget was requested from the UDC driver yet. I was wrong... usb_gadget_disconnect does not care about NULL pointer passed. :/ Best regards, Vitaly. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] usb_ether: register usb ethernet gadget at each eth init
Hi Lei, Lei Wen wrote: Since the ether may not be the only one usb gadget would be used in the uboot, it is neccessary to do the register each time the eth begin to work to make usb gadget driver less confussed when we want to use two different usb gadget at the same time. [...] @@ -1788,6 +1788,8 @@ static int usb_eth_init(struct eth_device *netdev, bd_t *bd) error(received NULL ptr); goto fail; } + if (usb_gadget_register_driver(eth_driver) 0) + goto fail; You probably need then at least to remove usb_gadget_register_driver call from usb_eth_initialize. And add usb_gadget_unregister_driver in usb_eth_halt. For example, my UDC driver returns EBUSY if another gadget driver tries to register itself. (Yes, I do not want to have more than 1 gadget). With your patch the gadget driver will fail each time when initializing. Anyway if I fix my UDC driver some day the Ether gadget will try to register itself twice (and more times later). So I will have to protect it against this bad behavior. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Pull request: u-boot-usb
Hi Remy, Remy Bohmer wrote: The following changes since commit ff377b1c0e891569b6da13629090aad7c38175e0: Wolfgang Denk (1): canmb board: Fix compiler warnings are available in the git repository at: git://git.denx.de/u-boot-usb.git next Just FYI There 2 bugfixes I would like to send soon (within 2 days). They fix some memory leaks and endpoint queue list corruption. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3] USB-CDC: Fix coding style issues
Hello Remy, vkuzmic...@mvista.com wrote: [...] - /* use PKTSIZE (or aligned... from u-boot) and set + /* + * use PKTSIZE (or aligned... from u-boot) and set * wMaxSegmentSize accordingly*/ Could you please fix the last line before commit? I missed this and would not like to spam more patches to the list. With best regards, Vitaly. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] USB-CDC: Fix coding style issues
Hello, Wolfgang Denk wrote: Dear Remy Bohmer, In message aanlkti=ptzmysnemx7zsrzowra7zhpaewbj92-2ae...@mail.gmail.com you wrote: Apart from that this patch looks good. Please fix globally. Hm... should we not rather try to keep (or bring) this file in sync with the corresponding current Linux header file? Yes, we should. In linux this comment was moved into previous: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=41dceed51f0e6105ca2bf45c3835a7cd9eaa077b#patch3 I'm going to fix this and add more details in the header. With best regards, Vitaly Kuzmichev. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout
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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/8] USB: gadget: fix porting bugs
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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] USB-CDC: wrong ep status used
Hi Sergei, On 08/12/2010 09:05 PM, Sergei Shtylyov wrote: Hello. Stefano Babic wrote: We get oops here! Agree, and the issue is not related to this patch, I missed to correct it, thanks. If no one complains, I will send a single patch to fix both problems (wrong ep status + null pointer access). Looks like Vitaly has done the same already: Yes, but I have not seen Stefano's comment when sent. Stefano, I would like to add you 'signed-off' in this patch because I took this part from your patch: - usb_ep_free_request (gadget-ep0, dev-req); + usb_ep_free_request (dev-status_ep, dev-req); Do you allow me to add your signature? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout
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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros
Replace Linux-like debug printout macros by native ones.
Signed-off-by: Vitaly Kuzmichev vkuzmic...@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__
[U-Boot] [PATCH v2 4/8] USB-CDC: Correct freeing usb requests
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 vkuzmic...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 5/8] USB-CDC: Replace 'strcpy' by 'strlcpy'
Replace 'strcpy' by more safe 'strlcpy' that is implemented in ether.c
Signed-off-by: Vitaly Kuzmichev vkuzmic...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 6/8] USB-CDC: Correct stat_req initialization
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 vkuzmic...@mvista.com
Signed-off-by: Stefano Babic sba...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 7/8] USB-CDC: ethernet error path potential oops fix
Fix potential oops on rare error path.
The patch is based on commit e7b13ec9235b9fded90f826ceeb8c34548631351
(done by David Brownell davi...@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 vkuzmic...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 8/8] USB-CDC: change simple_strtol to simple_strtoul
The patch is based on commit bb9496c6f7e853e5d4edd5397c9d45f1968d623c
(done by Julia Lawall ju...@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 vkuzmic...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros
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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros
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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/8] USB-CDC: Use native debug printout macros
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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] USB-CDC: wrong ep status used
Hi Stefano,
On 08/12/2010 01:41 AM, Stefano Babic wrote:
#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;
We get oops here!
- 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/8] USB-CDC: Restuct USB gadget Makefile
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 vkuzmic...@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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 4/8] USB-CDC: Correct freeing usb requests
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 vkuzmic...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 3/8] USB-CDC: Linux-like debug printout
Take debug printout macros back from linux-2.6.27 and make them more
useful and more compatible.
Signed-off-by: Vitaly Kuzmichev vkuzmic...@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
[U-Boot] [PATCH 5/8] USB-CDC: Replace 'strcpy' by 'strlcpy'
Replace 'strcpy' by more safe 'strlcpy' that is implemented in ether.c
Signed-off-by: Vitaly Kuzmichev vkuzmic...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 0/8] USB: gadget: fix porting bugs
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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 7/8] USB: gadget: ethernet error path potential oops fix
From: David Brownell davi...@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 dbrown...@users.sourceforge.net
Signed-off-by: Greg Kroah-Hartman gre...@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 vkuzmic...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 6/8] USB-CDC: Correct stat_req initialization
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 vkuzmic...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 8/8] USB: gadget: change simple_strtol to simple_strtoul
From: Julia Lawall ju...@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_str...@p(...)
@@
position p != r2.p;
type T;
T e;
@@
e =
- simple_str...@p
+ simple_strtoul
(...)
// /smpl
Signed-off-by: Julia Lawall ju...@diku.dk
Acked-by: David Brownell dbrown...@users.sourceforge.net
Signed-off-by: Greg Kroah-Hartman gre...@suse.de
(cherry picked from commit bb9496c6f7e853e5d4edd5397c9d45f1968d623c)
Signed-off-by: Vitaly Kuzmichev vkuzmic...@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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] USB-CDC with musb controller
Hi Remy,
I also have some fixes for ether.c.
How should I send them for review?
Through this mailing list or make 'git push' into private branch to
u-boot-usb.git repo?
On 08/11/2010 02:40 PM, Stefano Babic wrote:
Remy Bohmer wrote:
Hi Stefano,
Hi Remy,
Indeed, this seems to be some debug logging that can be removed...
Ok, understood.
We have tested it with the Atmel at91sam9261 core, and I have never
used it with any other hardware.
It is possible that you run into problems here that where not visible
on the Atmel core...
yes, this is what I have seen...
I report the whole log, enabling debug output on both musb driver and
ether.c. Do you have some hints to give me to go further ?
This is my log when I do a (successful) ping to the host when I run on
a Atmel eval-kit.
Maybe you can use it as reference? Maybe it helps...
Thanks a lot, it really helped me ;-).
At least, I can now identify what a real problem is and what is not..
I can now complete the setup phase and the interface is working.
I have found a couple of problems in ether.c that I will report to you.
Of course, if you agree with my analyses, I will send patches to fix them.
1. The status_req buffer is static allocated as u8. However, in
eth_status_complete is referenced with a 32 bit pointer:
__le32 *data = req-buf
In most case the buffer is not 32-bit aligned and causes an exception.
2. In eth_bind a wrong ep is allocated.
#if defined(DEV_CONFIG_CDC)
if (dev-status_ep) {
dev-stat_req = usb_ep_alloc_request(gadget-ep0,
GFP_KERNEL);
This should be:
dev-stat_req = usb_ep_alloc_request(dev-status_ep, GFP_KERNEL);
3. Not sure about the handling in usb_eth_send. I do not know if the fix
I propone works only for the musb driver or could be general and it was
to me not clear as the packet_sent variable is managed:
1834 while(!packet_sent)
1835 {
1836 packet_sent=0;
1837 }
It seems there is no possibility to change packet_sent if we run in the
loop
I managed to call handle_interrupts() inside the loop to get it working.
I can only assume that on your Atmel Core, tx_complete is called
directly after running into usb_ep_queue, and then you have not this issue.
But for most drivers, it should be required to call the interrupt
routine (or something like that, but we have already
handle_interrupts()) to manage all events.
Best regards,
Stefano
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] USB CDC branch
Hi Remi, What do you mean with ported to 2010.03? It was already working on latest git, do you mean you backported it to an older version? I mean that I have to work over our fork of main u-boot 2010.03. Of course before submitting the patches I will test them on both 'master' branches of main u-boot and u-boot-usb. Is RNDIS working stable? I didn't make tests yet. We had quite some troubles with it, especially with BSODs on those Windows boxes... Looks like I have to fix this :) The device unplug/plug behaviour was a pain for RNDIS, Do you mean that Windows and u-boot do not recognize cable unplug? But, I do not want to scare you off from this rndis work I have no choice :) We use it to communicate with a CDC driver on a Windows XP-embedded host (http://www.thesycon.de/eng/usb_cdcecm.shtml). Fine. And thanks for hint. ___ WBR, Vitaly. On 07/13/2010 11:48 PM, Remy Bohmer wrote: Hi, 2010/7/13 Vitaly Kuzmichev vkuzmic...@mvista.com: FYI I'm doing some work on this branch too. Great! Actually I have ported it to 2010.03 What do you mean with ported to 2010.03? It was already working on latest git, do you mean you backported it to an older version? (I would not expect this would be a big difference though) added RNDIS support Is RNDIS working stable? We had quite some troubles with it, especially with BSODs on those Windows boxes... The device unplug/plug behaviour was a pain for RNDIS, hence the reason why we removed that from the code. But, I do not want to scare you off from this rndis work, please continue. Maybe you can get it to work properly :-) We use it to communicate with a CDC driver on a Windows XP-embedded host (http://www.thesycon.de/eng/usb_cdcecm.shtml). CDC works much more stable, and much more faster compared to rndis. and made integration into 'usb' command. Great! However these changes aren't tested yet so the work is still under progress. Probably someone knows about any possible issues with CDC/RNDIS support? See above. and CDC is working stable, no known issues with it. At least one hint: First get CDC working on your hardware before you start to connect rndis, that would allow you to get the udc driver and all other levels of SW working properly and that you would only need to focus on rndis protocol rarities after that. And to prevent that you are debugging all kinds of undocumented rndis calls from the Windows host you could use a Linux host instead with the Linux-rndis-usb-host driver ;-) And also I would like to know the answers to Stefano's questions. See my other reply. I would suggest to merge all work in progress in the u-boot-usb/CDC branch from which we push it to mainline. Kind regards, Remy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] USB CDC branch
Hi, We also implemented the CDC and RNDIS on our board and working well. If you merge your usb gadget tree then we will send the patches also. Have you shared this anywhere (public git?)? I would like to look at your patches to sync them with mine. ___ WBR, Vitaly. On 07/14/2010 11:10 AM, Kyungmin Park wrote: On Wed, Jul 14, 2010 at 4:48 AM, Remy Bohmer li...@bohmer.net wrote: Hi, 2010/7/13 Vitaly Kuzmichev vkuzmic...@mvista.com: FYI I'm doing some work on this branch too. Great! Actually I have ported it to 2010.03 What do you mean with ported to 2010.03? It was already working on latest git, do you mean you backported it to an older version? (I would not expect this would be a big difference though) added RNDIS support Is RNDIS working stable? We had quite some troubles with it, especially with BSODs on those Windows boxes... The device unplug/plug behaviour was a pain for RNDIS, hence the reason why we removed that from the code. But, I do not want to scare you off from this rndis work, please continue. Maybe you can get it to work properly :-) We use it to communicate with a CDC driver on a Windows XP-embedded host (http://www.thesycon.de/eng/usb_cdcecm.shtml). CDC works much more stable, and much more faster compared to rndis. We also implemented the CDC and RNDIS on our board and working well. If you merge your usb gadget tree then we will send the patches also. To Minkyu, Can you prepare the patches and send to u-boot? Thank you, Kyungmin Park and made integration into 'usb' command. Great! However these changes aren't tested yet so the work is still under progress. Probably someone knows about any possible issues with CDC/RNDIS support? See above. and CDC is working stable, no known issues with it. At least one hint: First get CDC working on your hardware before you start to connect rndis, that would allow you to get the udc driver and all other levels of SW working properly and that you would only need to focus on rndis protocol rarities after that. And to prevent that you are debugging all kinds of undocumented rndis calls from the Windows host you could use a Linux host instead with the Linux-rndis-usb-host driver ;-) And also I would like to know the answers to Stefano's questions. See my other reply. I would suggest to merge all work in progress in the u-boot-usb/CDC branch from which we push it to mainline. Kind regards, Remy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] USB CDC branch
FYI I'm doing some work on this branch too. Actually I have ported it to 2010.03, added RNDIS support and made integration into 'usb' command. However these changes aren't tested yet so the work is still under progress. Probably someone knows about any possible issues with CDC/RNDIS support? And also I would like to know the answers to Stefano's questions. On 07/13/2010 11:26 AM, Stefano Babic wrote: Hi Remy, I would like to add support for Ethernet over USB (CDC) for the musb (musb_udc) controller, basing on your cdc branch in u-boot-usb. I have seen there are some conflicts and some duplicated structure between the header imported from linux (linux/usb/ch9.h) and the headers in u-boot (usbdescriptors.h, for example), and I have started to solve these issues. However, before doing some unnecessary work, I prefer to ask you which are your plans about this branch and if you think it could be merged soon to the u-boot-usb master and, at the end, brought to the mainline. Regards, Stefano ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] ARM: Align stack to 8 bytes
I would like to add some explanations: This is the issue gone from GCC behavior on VLA allocation. I did a simple test with VLA, and the following snippet from its ASM listing may clarify the root cause of issue: VLA allocation start. R1 is initialized by the length of VLA. 80080030: e281300fadd r3, r1, #15 ; 0xf 80080034: e2033f7eand r3, r3, #504; 0x1f8 Align VLA size. 80080038: e1a0500dmov r5, sp Save SP to recover it when VLA becomes needless. 8008003c: e04dd003sub sp, sp, r3 Allocate R3 bytes on stack. 80080040: e1a0300dmov r3, sp Store VLA address in R3. 80080044: e1a0c1a3lsr ip, r3, #3 80080048: e1a0218clsl r2, ip, #3 Here VLA address is aligned by 8 bytes. If SP is either 0xYYY4 or 0xZZZC, r2 will lose significant digit and will become 0xYYY0/0xZZZ8 (VLA=SP-4). It will less than SP, so the next 'push' (alias to STMDB) will decrement SP by 4 and will store register at the top of the stack, so this will overwrite first 4 bytes of VLA. On 06/15/2010 10:18 PM, Vitaly Kuzmichev wrote: The ARM ABI requires that the stack be aligned to 8 bytes as it is noted in Procedure Call Standard for the ARM Architecture: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042d/index.html Unaligned SP also causes the problem with variable-length arrays allocation when VLA address becomes less than stack pointer during aligning of this address, so the next 'push' in the stack overwrites first 4 bytes of VLA. Signed-off-by: Vitaly Kuzmichev vkuzmic...@mvista.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] ARM1136: Align stack to 8 bytes
The ARM ABI requires that the stack be aligned to 8 bytes as it is noted in Procedure Call Standard for the ARM Architecture: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042d/index.html Unaligned SP also causes the problem with variable-length arrays allocation when VLA address becomes less than stack pointer during aligning of this address, so the further 'push' into the stack overwrites first 4 bytes of VLA. Signed-off-by: Vitaly Kuzmichev vkuzmic...@mvista.com Signed-off-by: George G. Davis gda...@mvista.com --- arch/arm/cpu/arm1136/start.S |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index 957f438..d0c5717 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S @@ -185,6 +185,7 @@ stack_setup: #endif sub sp, r0, #12 /* leave 3 words for abort-stack*/ #endif /* CONFIG_PRELOADER */ + bic sp, sp, #7 /* 8-byte alignment */ clear_bss: ldr r0, _bss_start /* find start of bss segment*/ -- 1.6.2.3.127.gb136 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] ARM1136: Align stack to 8 bytes
Ok, but probably other ARM starters should be fixed too? I have no possibility to check all ARM CPUs, but I believe this patch won't break them. On 06/15/2010 03:16 PM, Martin Krause wrote: Hi Vitaly, the exact same problem applies to ARM1176. Maybe you could update your patch and add the same line to /arch/arm/cpu/arm1176/start.S. I am currently not working on an top-of-tree U-Boot, so it would be not so easy for me to create a separate patch for ARM1176. Regards, Martin u-boot-boun...@lists.denx.de wrote on Tuesday, June 15, 2010 12:52 PM: The ARM ABI requires that the stack be aligned to 8 bytes as it is noted in Procedure Call Standard for the ARM Architecture: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042d/index.html Unaligned SP also causes the problem with variable-length arrays allocation when VLA address becomes less than stack pointer during aligning of this address, so the further 'push' into the stack overwrites first 4 bytes of VLA. Signed-off-by: Vitaly Kuzmichev vkuzmic...@mvista.com Signed-off-by: George G. Davis gda...@mvista.com --- arch/arm/cpu/arm1136/start.S |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index 957f438..d0c5717 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S @@ -185,6 +185,7 @@ stack_setup: #endif sub sp, r0, #12 /* leave 3 words for abort-stack*/ #endif /* CONFIG_PRELOADER */ +bic sp, sp, #7 /* 8-byte alignment */ clear_bss: ldr r0, _bss_start /* find start of bss segment*/ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2] ARM: Align stack to 8 bytes
The ARM ABI requires that the stack be aligned to 8 bytes as it is noted in Procedure Call Standard for the ARM Architecture: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042d/index.html Unaligned SP also causes the problem with variable-length arrays allocation when VLA address becomes less than stack pointer during aligning of this address, so the next 'push' in the stack overwrites first 4 bytes of VLA. Signed-off-by: Vitaly Kuzmichev vkuzmic...@mvista.com --- arch/arm/cpu/arm1136/start.S |1 + arch/arm/cpu/arm1176/start.S |1 + arch/arm/cpu/arm720t/start.S |1 + arch/arm/cpu/arm920t/start.S |1 + arch/arm/cpu/arm925t/start.S |1 + arch/arm/cpu/arm926ejs/start.S|2 +- arch/arm/cpu/arm946es/start.S |1 + arch/arm/cpu/arm_cortexa8/start.S |2 +- arch/arm/cpu/arm_intcm/start.S|1 + arch/arm/cpu/ixp/start.S |1 + arch/arm/cpu/lh7a40x/start.S |1 + arch/arm/cpu/pxa/start.S |1 + arch/arm/cpu/s3c44b0/start.S |1 + arch/arm/cpu/sa1100/start.S |1 + 14 files changed, 14 insertions(+), 2 deletions(-) diff --git a/arch/arm/cpu/arm1136/start.S b/arch/arm/cpu/arm1136/start.S index 957f438..f0cb3cc 100644 --- a/arch/arm/cpu/arm1136/start.S +++ b/arch/arm/cpu/arm1136/start.S @@ -185,6 +185,7 @@ stack_setup: #endif sub sp, r0, #12 /* leave 3 words for abort-stack*/ #endif /* CONFIG_PRELOADER */ + bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ clear_bss: ldr r0, _bss_start /* find start of bss segment*/ diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S index e2b6c9b..2162a6b 100644 --- a/arch/arm/cpu/arm1176/start.S +++ b/arch/arm/cpu/arm1176/start.S @@ -249,6 +249,7 @@ stack_setup: sub r0, r0, #CONFIG_SYS_MALLOC_LEN /* malloc area */ sub r0, r0, #CONFIG_SYS_GBL_DATA_SIZE /* bdinfo */ sub sp, r0, #12 /* leave 3 words for abort-stack*/ + bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ clear_bss: ldr r0, _bss_start /* find start of bss segment*/ diff --git a/arch/arm/cpu/arm720t/start.S b/arch/arm/cpu/arm720t/start.S index 022b873..d6f2c16 100644 --- a/arch/arm/cpu/arm720t/start.S +++ b/arch/arm/cpu/arm720t/start.S @@ -172,6 +172,7 @@ stack_setup: sub r0, r0, #(CONFIG_STACKSIZE_IRQ+CONFIG_STACKSIZE_FIQ) #endif sub sp, r0, #12 /* leave 3 words for abort-stack*/ + bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ clear_bss: ldr r0, _bss_start /* find start of bss segment*/ diff --git a/arch/arm/cpu/arm920t/start.S b/arch/arm/cpu/arm920t/start.S index 779f192..e532f55 100644 --- a/arch/arm/cpu/arm920t/start.S +++ b/arch/arm/cpu/arm920t/start.S @@ -204,6 +204,7 @@ stack_setup: sub r0, r0, #(CONFIG_STACKSIZE_IRQ+CONFIG_STACKSIZE_FIQ) #endif sub sp, r0, #12 /* leave 3 words for abort-stack*/ + bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ clear_bss: ldr r0, _bss_start /* find start of bss segment*/ diff --git a/arch/arm/cpu/arm925t/start.S b/arch/arm/cpu/arm925t/start.S index 567e804..346615e 100644 --- a/arch/arm/cpu/arm925t/start.S +++ b/arch/arm/cpu/arm925t/start.S @@ -196,6 +196,7 @@ stack_setup: sub r0, r0, #(CONFIG_STACKSIZE_IRQ+CONFIG_STACKSIZE_FIQ) #endif sub sp, r0, #12 /* leave 3 words for abort-stack*/ + bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ clear_bss: ldr r0, _bss_start /* find start of bss segment*/ diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S index 3b81151..cf40ce1 100644 --- a/arch/arm/cpu/arm926ejs/start.S +++ b/arch/arm/cpu/arm926ejs/start.S @@ -196,7 +196,7 @@ stack_setup: #endif #endif /* CONFIG_PRELOADER */ sub sp, r0, #12 /* leave 3 words for abort-stack*/ - bic sp, r0, #7 /* 8-byte align stack for ABI compliance */ + bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ clear_bss: ldr r0, _bss_start /* find start of bss segment*/ diff --git a/arch/arm/cpu/arm946es/start.S b/arch/arm/cpu/arm946es/start.S index 627e3cb..8844d44 100644 --- a/arch/arm/cpu/arm946es/start.S +++ b/arch/arm/cpu/arm946es/start.S @@ -163,6 +163,7 @@ stack_setup: sub r0, r0, #(CONFIG_STACKSIZE_IRQ+CONFIG_STACKSIZE_FIQ) #endif sub sp, r0, #12 /* leave 3 words for abort-stack*/ + bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ clear_bss: ldr r0, _bss_start
