Skip to contentSkip to conversation

updated Patch to option HSO driver to the kernel

RFC In review Greg KH 9 participants 22 messages Apr 17, 2008 – Apr 28, 2008 14 messages not in the archive

Subsystems (4)
Networking Network drivers USB networking drivers Hso 3G modem driver
lore
Share & tools
lore

Greg KH

author
#0 lore raw

RFC 1/1 updated Patch to option HSO driver to the kernel

Thanks for all of the help with this, here's the latest version of the driver so that everyone can sync up with it. I removed a lot of the function declarations by moving some of the code around, but I think more can still be done.

I'm going to be with shaky email access for the next week, due to cross-atlantic travel and conference fun, so my responses might be a bit slow.

thanks,

greg k-h

--------------
Subject: USB: add option hso driver

This driver is for a number of different Option devices. Originally written by Option and Andrew Bird, but cleaned up massivly for acceptance into mainline by me (Greg).

TODO:
- remove proc files and move to debugfs
- review network interfaces
- add better changelog information
- Use netif_msg_ for the message level rather than module parameter
- net_device_stats are now available in dev->stats

Many thanks to the following for their help in cleaning up the driver by providing feedback and patches to it:
- Paulius Zaleckas <[email protected]>
- Oliver Neukum <[email protected]>
- Alan Cox <alan-qBU/[email protected]>

Cc Andrew Bird <ajb-5+cxppFmGx6/3pe1ocb+s/[email protected]>
Cc Alan Cox <alan-qBU/[email protected]>
Cc Filip Aben <[email protected]>
Cc Paulius Zaleckas <[email protected]>
Cc Oliver Neukum <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/net/usb/Kconfig  |   10 
 drivers/net/usb/Makefile |    1 
 drivers/net/usb/hso.c    | 3153 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 3164 insertions(+)
 create mode 100644 drivers/net/usb/hso.c
drivers/net/usb/Kconfig +10 -0
@@ -154,6 +154,16 @@ config USB_NET_AX8817X
 	  This driver creates an interface named "ethX", where X depends on
 	  what other networking devices you have in use.
 
+config USB_HSO
+	tristate "Option USB High Speed Mobile Devices"
+	depends on USB
+	default n
+	help
+	  Choose this option if you have an Option HSDPA/HSUPA card.
+	  These cards support downlink speeds of 7.2Mbps or greater.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called hso.
 
 config USB_NET_CDCETHER
 	tristate "CDC Ethernet support (smart devices such as cable modems)"
drivers/net/usb/Makefile +1 -0
@@ -6,6 +6,7 @@ obj-$(CONFIG_USB_CATC)		+= catc.o
 obj-$(CONFIG_USB_KAWETH)	+= kaweth.o
 obj-$(CONFIG_USB_PEGASUS)	+= pegasus.o
 obj-$(CONFIG_USB_RTL8150)	+= rtl8150.o
+obj-$(CONFIG_USB_HSO)		+= hso.o
 obj-$(CONFIG_USB_NET_AX8817X)	+= asix.o
 obj-$(CONFIG_USB_NET_CDCETHER)	+= cdc_ether.o
 obj-$(CONFIG_USB_NET_DM9601)	+= dm9601.o
drivers/net/usb/hso.c new file +3153 -0

2,977 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Part of the diff is not shown here. View the raw message.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw

- Cleaned up unused defines and variables
- Remove broken firmware info from ethtool_ops
- Const for usb IDs table

Signed-off-by Paulius Zaleckas <[email protected]>
drivers/net/usb/hso.c +3 -43
@@ -82,7 +82,6 @@
 #define BULK_URB_RX_SIZE		4096
 #define BULK_URB_TX_SIZE		8192
 
-#define MUX_INTR_BUF_SIZE		16
 #define MUX_BULK_RX_BUF_SIZE		HSO__MAX_MTU
 #define MUX_BULK_TX_BUF_SIZE		HSO__MAX_MTU
 #define MUX_BULK_RX_BUF_COUNT		4

@@ -91,18 +90,11 @@
 /* These definitions are used with the struct hso_net flags element */
 /* - use *_bit operations on it. (bit indices not values.) */
 #define HSO_NET_RUNNING			0
-#define HSO_NET_TX_BUSY			1
-#define HSO_TERMINATE			2
 
 #define	HSO_NET_TX_TIMEOUT		(HZ*10)
 
 /* Serial port defines and structs. */
-#define HSO_THRESHOLD_THROTTLE		(7*1024)
-#define HSO_THRESHOLD_UNTHROTTLE	(2*1024)
-
-#define HSO_SERIAL_FLAG_THROTTLED	0
-#define HSO_SERIAL_FLAG_TX_SENT		1
-#define HSO_SERIAL_FLAG_RX_SENT		2
+#define HSO_SERIAL_FLAG_RX_SENT		0
 
 #define HSO_SERIAL_MAGIC		0x48534f31
 

@@ -182,12 +174,8 @@ struct hso_net {
 	struct iphdr rx_ip_hdr;
 	struct ethhdr dummy_eth_head;
 
-	__u16 bcdCDC;
-	__u16 wMaxSegmentSize;
 	__u16 wNumberMCFilters;
-	__u16 mode_flags;
 	unsigned long flags;
-
 };
 
 struct hso_serial {

@@ -210,7 +198,6 @@ struct hso_serial {
 	u16 tx_data_length;	/* should contain allocated length */
 	u16 tx_data_count;
 	u16 tx_buffer_count;
-	spinlock_t buffer_lock;
 	struct usb_ctrlrequest ctrl_req_tx;
 	struct usb_ctrlrequest ctrl_req_rx;
 

@@ -226,7 +213,6 @@ struct hso_serial {
 	struct tty_struct *tty;
 	int open_count;
 	spinlock_t serial_lock;
-	void *private;
 
 	int (*write_data) (struct hso_serial *serial);
 };

@@ -240,7 +226,6 @@ struct hso_device {
 	u32 port_spec;
 
 	u8 is_active;
-	u8 suspend_disabled;
 	u8 usb_gone;
 	struct work_struct async_get_intf;
 	struct work_struct async_put_intf;

@@ -252,9 +237,6 @@ struct hso_device {
 	struct device *dev;
 	struct kref ref;
 	struct mutex mutex;
-
-	/* TODO: Not sure about the ones below */
-	struct proc_dir_entry *ourproc;
 };
 
 /* Type of interface */

@@ -327,8 +309,6 @@ static unsigned char hex2dec(unsigned ch
 	return 0;
 }
 
-#define SIOCSETRADIO   (SIOCDEVPRIVATE+4)
-
 /* #define DEBUG */
 
 #define dev2net(x) (x->port_data.dev_net)

@@ -411,7 +391,7 @@ static const s32 icon321_port_spec[] = {
 		.driver_info = (kernel_ulong_t)icon321_port_spec
 
 /* list of devices we support */
-static struct usb_device_id hso_ids[] = {
+static const struct usb_device_id hso_ids[] = {
 	{default_port_device(0x0af0, 0x6711)},
 	{default_port_device(0x0af0, 0x6731)},
 	{default_port_device(0x0af0, 0x6751)},

@@ -610,7 +590,7 @@ static void set_serial_by_index(unsigned
 	spin_unlock_irqrestore(&serial_table_lock, flags);
 }
 
-/* log a meaningfull explanation of an USB status to syslog */
+/* log a meaningfull explanation of an USB status */
 static void log_usb_status(int status, const char *function)
 {
 	char *explanation = NULL;

@@ -858,8 +838,6 @@ static void hso_get_drvinfo(struct net_d
 	strncpy(info->driver, driver_name, ETHTOOL_BUSINFO_LEN);
 	strncpy(info->version, DRIVER_VERSION, ETHTOOL_BUSINFO_LEN);
 	usb_make_path(odev->parent->usb, info->bus_info, sizeof info->bus_info);
-	snprintf(info->fw_version, ETHTOOL_BUSINFO_LEN, "%s %x.%x", driver_name,
-		 ((odev->bcdCDC & 0xff00) >> 8), (odev->bcdCDC & 0x00ff));
 }
 
 static struct ethtool_ops ops = {

@@ -907,34 +885,17 @@ static void hso_net_set_multicast(struct
 	if (net->flags & IFF_PROMISC) {
 		/* Unconditionally log net taps. */
 		D1("%s: Promiscuous mode enabled", net->name);
-		odev->mode_flags = USB_CDC_PACKET_TYPE_ALL_MULTICAST |
-				   USB_CDC_PACKET_TYPE_DIRECTED |
-				   USB_CDC_PACKET_TYPE_BROADCAST |
-				   USB_CDC_PACKET_TYPE_MULTICAST |
-				   USB_CDC_PACKET_TYPE_PROMISCUOUS;
 	} else if (net->mc_count > odev->wNumberMCFilters) {
 		/* Too many to filter perfectly -- accept all multicasts. */
 		D1("%s: too many MC filters for hardware, using allmulti",
 		   net->name);
-		odev->mode_flags = USB_CDC_PACKET_TYPE_ALL_MULTICAST |
-				   USB_CDC_PACKET_TYPE_DIRECTED |
-				   USB_CDC_PACKET_TYPE_BROADCAST |
-				   USB_CDC_PACKET_TYPE_MULTICAST;
 	} else if (net->flags & IFF_ALLMULTI) {
 		/* Filter in software */
 		D1("%s: using allmulti", net->name);
-		odev->mode_flags = USB_CDC_PACKET_TYPE_ALL_MULTICAST |
-				   USB_CDC_PACKET_TYPE_DIRECTED |
-				   USB_CDC_PACKET_TYPE_BROADCAST |
-				   USB_CDC_PACKET_TYPE_MULTICAST;
 	} else {
 		/* do multicast filtering in hardware */
 		struct dev_mc_list *mclist;
 		D1("%s: set multicast filters", net->name);
-		odev->mode_flags = USB_CDC_PACKET_TYPE_ALL_MULTICAST |
-				   USB_CDC_PACKET_TYPE_DIRECTED |
-				   USB_CDC_PACKET_TYPE_BROADCAST |
-				   USB_CDC_PACKET_TYPE_MULTICAST;
 		buf = kmalloc(6 * net->mc_count, GFP_ATOMIC);
 		if (!buf) {
 			dev_err(&net->dev, "No memory to allocate?");

@@ -2213,7 +2174,6 @@ static void hso_net_init(struct net_devi
 	SET_ETHTOOL_OPS(net, &ops);
 	hso_net->skb_rx_buf = NULL;
 	hso_net->rx_parse_state = WAIT_IP;
-	hso_net->wMaxSegmentSize = DEFAULT_MTU;
 	/* always use all multi, no lists of multicasts */
 	hso_net->wNumberMCFilters = 0;
#3 lore raw

Am Montag, 21. April 2008 03:04:21 schrieb Paulius Zaleckas:

Quoted 4 lines

-               odev->mode_flags = USB_CDC_PACKET_TYPE_ALL_MULTICAST | -                                  USB_CDC_PACKET_TYPE_DIRECTED | -                                  USB_CDC_PACKET_TYPE_BROADCAST | -                                  USB_CDC_PACKET_TYPE_MULTICAST;

Hi,

before you rip out the multicast stuff, do you have an idea why it was included in the first place? Is the device capable of operating in a real network topology where multicast would make sense?

Regards
Oliver

#4 lore raw
Quoted 11 lines · Oliver Neukum

Am Montag, 21. April 2008 03:04:21 schrieb Paulius Zaleckas:

drivers/net/usb/hso.c static void hso_net_set_multicast(struct · line 922

Quoted 4 lines
drivers/net/usb/hso.c +0 -4
-               odev->mode_flags = USB_CDC_PACKET_TYPE_ALL_MULTICAST |
-                                  USB_CDC_PACKET_TYPE_DIRECTED |
-                                  USB_CDC_PACKET_TYPE_BROADCAST |
-                                  USB_CDC_PACKET_TYPE_MULTICAST;

Hi,

before you rip out the multicast stuff, do you have an idea why it was included in the first place? Is the device capable of operating in a real network topology where multicast would make sense?

It was there from the oldest version (0.6) I have, but in the dead code as it is now. There was also a comment:
/* FIXME AJB, work out what we need to do with multicast */ It was addressed to Andrew Bird (added to CC)

IMO it doesn't make sense to do multicasting since actually it is point-to-point and not ethernet traffic.
Currently I am studying if it is possible to convert current pseudo ethernet interface to point-to-point one.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw
Quoted 7 lines

From: Paulius Zaleckas <[email protected]>
Date: Mon, 21 Apr 2008 12:34:39 +0300

IMO it doesn't make sense to do multicasting since actually it is point-to-point and not ethernet traffic.
Currently I am studying if it is possible to convert current pseudo ethernet interface to point-to-point one.

For the record: when I looked at that issue with respect to USB network connections for handheld devices, one issue seemed to be weak support for managing (lots of) point-to-point links.

The example that sticks in my memory is that of routing. Setting up a point to point link between a handheld and a PC required the PC to act as an IP router. Managing lots of desktop routers in any large configuration seemed to be a losing game ... and one that many network administrators would refuse to play. And that's in addition to the lack of a dynamic IP address assignment solution for lots of point-to-point links.

The alternative was to set up the links like Ethernet, then bridge them. That's a *much* more manageable solution, even though I still don't know of any distro that makes bridge setup work as easily as Windows does (sigh). Bridging properly requires the ability to handle multicast and broadcast packets ... and once you've got that then DHCP, zeroconf, and other automatic network configuration schemes work easily.

QED ... that's why the "usbnet" framework doesn't try to use the point-to-point framework, instead allows N-casting.

- Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20080421184157.106603602C8-ZcXrCSuhvln6VZ3dlLfH/[email protected]

#8 lore raw
Quoted 28 lines · David Brownell
Quoted 7 lines

From: Paulius Zaleckas <[email protected]>
Date: Mon, 21 Apr 2008 12:34:39 +0300

IMO it doesn't make sense to do multicasting since actually it is point-to-point and not ethernet traffic.
Currently I am studying if it is possible to convert current pseudo ethernet interface to point-to-point one.

For the record: when I looked at that issue with respect to USB network connections for handheld devices, one issue seemed to be weak support for managing (lots of) point-to-point links.

The example that sticks in my memory is that of routing. Setting up a point to point link between a handheld and a PC required the PC to act as an IP router. Managing lots of desktop routers in any large configuration seemed to be a losing game ... and one that many network administrators would refuse to play. And that's in addition to the lack of a dynamic IP address assignment solution for lots of point-to-point links.

The alternative was to set up the links like Ethernet, then bridge them. That's a *much* more manageable solution, even though I still don't know of any distro that makes bridge setup work as easily as Windows does (sigh). Bridging properly requires the ability to handle multicast and broadcast packets ... and once you've got that then DHCP, zeroconf, and other automatic network configuration schemes work easily.

Quoted 4 lines

QED ... that's why the "usbnet" framework doesn't try to use the point-to-point framework, instead allows N-casting.

- Dave

I would agree with you, but... To make this interface work you have to send AT command with APN and optionally with PAP/CHAP username and password. Then you call AT command witch initiates PPP with basestation and begins to send/receive traffic on this pseudo ethernet interface... Actually it still needs one more AT command to get your IP and DNS addresses. Then you do: ifconfig hso0 xxx.xxx.xxx.xxx netmask 255.255.255.255 up And then you have to write DNS addresses (got through AT interface) to the /etc/resolv.conf. Now you have a working internet connection!

So... I don't see any chance to do multicasting or broadcasting here... IMO the only way to distribute this kind of connection to local network is to do NAT'ing :( Am I wrong?

P.S. Pseudo ethernet means that it receives bare IP packet from USB device, adds dummy ethernet header and sends it for further processing to kernel.

Paulius

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#9 lore raw
Quoted 8 lines · Paulius Zaleckas

To make this interface work you have to
send AT command with APN and optionally with PAP/CHAP username and password. Then you call AT command witch initiates PPP with basestation and begins to send/receive traffic on this pseudo ethernet interface... Actually it still needs one more AT command to get your IP and DNS addresses. Then you do: ifconfig hso0 xxx.xxx.xxx.xxx netmask 255.255.255.255 up And then you have to write DNS addresses (got through AT interface) to the /etc/resolv.conf. Now you have a working internet connection!

Hardly a general purpose device, then ... not even the model I'm used to seeing from a DSL or cable modem. Presumably you have some tool that runs on Linux to set up links like that.

Quoted 1 line

So... I don't see any chance to do multicasting or broadcasting here...

Well, some folk would want IP multicast for video support, but that's a different issue.

- Dave

Quoted 2 lines

P.S. Pseudo ethernet means that it receives bare IP packet from USB device, adds dummy ethernet header and sends it for further processing to kerne

A message that is not in the archive (sent off-list or not yet received): [email protected]

#11 lore raw

Paulius Zaleckas schreef:

Quoted 43 lines
Quoted 28 lines · David Brownell
Quoted 7 lines

From: Paulius Zaleckas <[email protected]>
Date: Mon, 21 Apr 2008 12:34:39 +0300

IMO it doesn't make sense to do multicasting since actually it is point-to-point and not ethernet traffic.
Currently I am studying if it is possible to convert current pseudo ethernet interface to point-to-point one.

For the record: when I looked at that issue with respect to USB network connections for handheld devices, one issue seemed to be weak support for managing (lots of) point-to-point links.

The example that sticks in my memory is that of routing. Setting up a point to point link between a handheld and a PC required the PC to act as an IP router. Managing lots of desktop routers in any large configuration seemed to be a losing game ... and one that many network administrators would refuse to play. And that's in addition to the lack of a dynamic IP address assignment solution for lots of point-to-point links.

The alternative was to set up the links like Ethernet, then bridge them. That's a *much* more manageable solution, even though I still don't know of any distro that makes bridge setup work as easily as Windows does (sigh). Bridging properly requires the ability to handle multicast and broadcast packets ... and once you've got that then DHCP, zeroconf, and other automatic network configuration schemes work easily.

Quoted 4 lines

QED ... that's why the "usbnet" framework doesn't try to use the point-to-point framework, instead allows N-casting.

- Dave

I would agree with you, but... To make this interface work you have to send AT command with APN and optionally with PAP/CHAP username and password. Then you call AT command witch initiates PPP with basestation and begins to send/receive traffic on this pseudo ethernet interface... Actually it still needs one more AT command to get your IP and DNS addresses. Then you do: ifconfig hso0 xxx.xxx.xxx.xxx netmask 255.255.255.255 up And then you have to write DNS addresses (got through AT interface) to the /etc/resolv.conf. Now you have a working internet connection!

Quoted 6 lines

So... I don't see any chance to do multicasting or broadcasting here... IMO the only way to distribute this kind of connection to local network is to do NAT'ing :( Am I wrong?

P.S. Pseudo ethernet means that it receives bare IP packet from USB device, adds dummy ethernet header and sends it for further processing to kernel.

Correct. And we don't support DHCP.

Filip-
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#12 lore raw
In reply to a message not in the archive

Am Dienstag, 22. April 2008 00:04:11 schrieb Paulius Zaleckas:

Quoted 9 lines

I would agree with you, but... To make this interface work you have to send AT command with APN and optionally with PAP/CHAP username and password. Then you call AT command witch initiates PPP with basestation and begins to send/receive traffic on this pseudo ethernet interface... Actually it still needs one more AT command to get your IP and DNS addresses. Then you do: ifconfig hso0 xxx.xxx.xxx.xxx netmask 255.255.255.255 up And then you have to write DNS addresses (got through AT interface) to the /etc/resolv.conf. Now you have a working internet connection!

Quoted 3 lines

So... I don't see any chance to do multicasting or broadcasting here... IMO the only way to distribute this kind of connection to local network is to do NAT'ing :( Am I wrong?

But what in this setup would prevent the other end of the connection from sending packets to a multicast address down the link? If that can happen, the questions remains, can the device filter these packets in hardware?

Regards
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

#14 lore raw

Incremental patch.
- Remove multicasting

Signed-off-by Paulius Zaleckas <[email protected]>
drivers/net/usb/hso.c renamed from drivers/net/usb/hso.c.orig +0 -48
@@ -174,7 +174,6 @@ struct hso_net {
 	struct iphdr rx_ip_hdr;
 	struct ethhdr dummy_eth_head;
 
-	__u16 wNumberMCFilters;
 	unsigned long flags;
 };
 

@@ -870,49 +869,6 @@ static void hso_net_tx_timeout(struct ne
 	odev->stats.tx_errors++;
 }
 
-/* setup the multicast filters */
-static void hso_net_set_multicast(struct net_device *net)
-{
-	struct hso_net *odev = netdev_priv(net);
-	int i = 0;
-	__u8 *buf = NULL;
-
-	/* Tell the kernel to stop sending us frames while we get this all set
-	 * up. */
-	netif_stop_queue(net);
-
-	/* Note: do not reorder, GCC is clever about common statements. */
-	if (net->flags & IFF_PROMISC) {
-		/* Unconditionally log net taps. */
-		D1("%s: Promiscuous mode enabled", net->name);
-	} else if (net->mc_count > odev->wNumberMCFilters) {
-		/* Too many to filter perfectly -- accept all multicasts. */
-		D1("%s: too many MC filters for hardware, using allmulti",
-		   net->name);
-	} else if (net->flags & IFF_ALLMULTI) {
-		/* Filter in software */
-		D1("%s: using allmulti", net->name);
-	} else {
-		/* do multicast filtering in hardware */
-		struct dev_mc_list *mclist;
-		D1("%s: set multicast filters", net->name);
-		buf = kmalloc(6 * net->mc_count, GFP_ATOMIC);
-		if (!buf) {
-			dev_err(&net->dev, "No memory to allocate?");
-			goto exit;
-		}
-		for (i = 0, mclist = net->mc_list; mclist && i < net->mc_count;
-		     i++, mclist = mclist->next) {
-			memcpy(&mclist->dmi_addr, &buf[i * 6], 6);
-		}
-		kfree(buf);
-	}
-
-exit:
-	/* Tell the kernel to start giving frames to us again. */
-	netif_wake_queue(net);
-}
-
 /* make a real packet from the received USB buffer */
 static void packetizeRx(struct hso_net *odev, unsigned char *ip_pkt,
 			unsigned int count, unsigned char is_eop)

@@ -2071,7 +2027,6 @@ static int hso_serial_common_create(stru
 exit:
 	hso_serial_common_free(serial);
 	return -1;
-
 }
 
 /* Frees a general hso device */

@@ -2167,15 +2122,12 @@ static void hso_net_init(struct net_devi
 	net->get_stats = hso_net_get_stats;
 	net->tx_timeout = hso_net_tx_timeout;
 	net->watchdog_timeo = HSO_NET_TX_TIMEOUT;
-	net->set_multicast_list = hso_net_set_multicast;
 	net->flags |= IFF_NOARP;
 	net->mtu = DEFAULT_MTU - 14;
 	net->tx_queue_len = 10;
 	SET_ETHTOOL_OPS(net, &ops);
 	hso_net->skb_rx_buf = NULL;
 	hso_net->rx_parse_state = WAIT_IP;
-	/* always use all multi, no lists of multicasts */
-	hso_net->wNumberMCFilters = 0;
 
 	/* and initialize the semaphore */
 	spin_lock_init(&hso_net->net_lock);

@@ -3084,7 +3036,6 @@ static void __exit hso_exit(void)
 		remove_proc_entry("devices", hso_proc_dir);
 		remove_proc_entry(driver_name, NULL);
 	}

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): 200804211317.23971.david-b-yBeKhBN/[email protected]

#18 lore raw
Quoted 6 lines · David Brownell
Quoted 2 lines · Paulius Zaleckas

Incremental patch.
- Remove multicasting

Is it really a good idea to remove the ability to
use DHCP over these links?

IMO it is really good idea:
1. Please read my comments to your previous post.
2. This code was dead and broken anyway. If you don't believe just look what wNumberMCFilters variable is doing there :)

Paulius

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#19 lore raw
In reply to a message not in the archive

Incremental patch.
- use net_device_stats from dev->stats

drivers/net/usb/hso.c renamed from drivers/net/usb/hso.c.orig +8 -9
@@ -152,7 +152,6 @@ struct hso_shared_int {
 
 struct hso_net {
 	struct hso_device *parent;
-	struct net_device_stats stats;
 	struct net_device *net;
 
 	struct usb_endpoint_descriptor *in_endp;

@@ -712,7 +711,7 @@ static int hso_net_open(struct net_devic
 		return -ENODEV;
 	}
 	/* reset read counter. */
-	odev->stats.rx_packets = 0;
+	odev->net->stats.rx_packets = 0;
 	odev->skb_tx_buf = NULL;
 
 	/* setup environment */

@@ -817,11 +816,11 @@ static int hso_net_start_xmit(struct sk_
 	if (result) {
 		dev_warn(&odev->parent->interface->dev,
 			"failed mux_bulk_tx_urb %d", result);
-		odev->stats.tx_errors++;
+		odev->net->stats.tx_errors++;
 		netif_start_queue(net);
 	} else {
-		odev->stats.tx_packets++;
-		odev->stats.tx_bytes += skb->len;
+		odev->net->stats.tx_packets++;
+		odev->net->stats.tx_bytes += skb->len;
 		/* And tell the kernel when the last transmit started. */
 		net->trans_start = jiffies;
 	}

@@ -846,7 +845,7 @@ static struct ethtool_ops ops = {
 
 static struct net_device_stats *hso_net_get_stats(struct net_device *net)
 {
-	return &((struct hso_net *)netdev_priv(net))->stats;
+	return &net->stats;
 }
 
 /* called when a packet did not ack after watchdogtimeout */

@@ -866,7 +865,7 @@ static void hso_net_tx_timeout(struct ne
 		usb_unlink_urb(odev->mux_bulk_tx_urb);
 
 	/* Update statistics */
-	odev->stats.tx_errors++;
+	odev->net->stats.tx_errors++;
 }
 
 /* make a real packet from the received USB buffer */

@@ -991,11 +990,11 @@ static void packetizeRx(struct hso_net *
 				odev->skb_rx_buf = NULL;
 
 				/* update out statistics */
-				odev->stats.rx_packets++;
+				odev->net->stats.rx_packets++;
 
 				/* Hmmm, wonder if we have received the IP len
 				 * or the ETH len. */
-				odev->stats.rx_bytes += odev->rx_buf_size;
+				odev->net->stats.rx_bytes += odev->rx_buf_size;
 
 				odev->rx_buf_size = 0;
 				odev->rx_buf_missing = sizeof(struct iphdr);

A message that is not in the archive (sent off-list or not yet received): [email protected]

#21 lore raw

Incremental patch.
- remove not needed variable initializations
- remove hso_net_get_stats, because by default identical internal_stats function from net/core/dev.c is used.
- remove not needed memset(hso_net, 0, sizeof(*hso_net)), because memory is allocated with kzalloc

drivers/net/usb/hso.c renamed from drivers/net/usb/hso.c.orig +47 -57
@@ -547,7 +547,7 @@ static struct hso_serial *get_serial_by_
 
 static struct hso_serial *get_serial_by_index(unsigned index)
 {
-	struct hso_serial *serial = NULL;
+	struct hso_serial *serial;
 	unsigned long flags;
 
 	if (!serial_table[index])

@@ -561,7 +561,7 @@ static struct hso_serial *get_serial_by_
 
 static int get_free_serial_index(void)
 {
-	int index = 0;
+	int index;
 	unsigned long flags;
 
 	spin_lock_irqsave(&serial_table_lock, flags);

@@ -591,7 +591,7 @@ static void set_serial_by_index(unsigned
 /* log a meaningfull explanation of an USB status */
 static void log_usb_status(int status, const char *function)
 {
-	char *explanation = NULL;
+	char *explanation;
 
 	switch (status) {
 	case -ENODEV:

@@ -630,10 +630,9 @@ static void log_usb_status(int status, c
 static int hso_proc_options(char *buf, char **start, off_t offset, int count,
 			    int *eof, void *data)
 {
-	int len = 0;
+	int len;
 	/* get the module parameters */
-	len +=
-	    snprintf(buf + len, count - len, "Version: %s\n", DRIVER_VERSION);
+	len = snprintf(buf, count, "Version: %s\n", DRIVER_VERSION);
 	len += snprintf(buf + len, count - len, "debug: 0x%02x\n", debug);
 	len += snprintf(buf + len, count - len, "procfs: 0x%02x\n", procfs);
 	len +=

@@ -647,9 +646,9 @@ static int hso_proc_options(char *buf, c
 static int hso_proc_port_info(char *buf, char **start, off_t offset, int count,
 			      int *eof, void *data)
 {
-	int len = 0;
+	int len;
 	struct hso_device *hso_dev = (struct hso_device *)data;
-	char *port_name = NULL;
+	char *port_name;
 
 	D1("count: %d", count);
 

@@ -689,7 +688,7 @@ static int hso_proc_port_info(char *buf,
 		break;
 	}
 
-	len += snprintf(buf + len, count - len, "%s\n", port_name);
+	len = snprintf(buf, count, "%s\n", port_name);
 
 	/* return the device id to the user, for use in scripts for autopm */
 	len +=

@@ -782,7 +781,7 @@ static void write_bulk_callback(struct u
 static int hso_net_start_xmit(struct sk_buff *skb, struct net_device *net)
 {
 	struct hso_net *odev = netdev_priv(net);
-	int result = 0;
+	int result;
 
 	/* Tell the kernel, "No more frames 'til we are done with this one." */
 	netif_stop_queue(net);

@@ -843,11 +842,6 @@ static struct ethtool_ops ops = {
 	.get_link = ethtool_op_get_link
 };
 
-static struct net_device_stats *hso_net_get_stats(struct net_device *net)
-{
-	return &net->stats;
-}
-
 /* called when a packet did not ack after watchdogtimeout */
 static void hso_net_tx_timeout(struct net_device *net)
 {

@@ -872,11 +866,11 @@ static void hso_net_tx_timeout(struct ne
 static void packetizeRx(struct hso_net *odev, unsigned char *ip_pkt,
 			unsigned int count, unsigned char is_eop)
 {
-	unsigned short temp_bytes = 0;
+	unsigned short temp_bytes;
 	unsigned short buffer_offset = 0;
-	unsigned short frame_len = 0;
-	unsigned char *tmp_rx_buf = NULL;
-	struct ethhdr *eth_head = NULL;
+	unsigned short frame_len;
+	unsigned char *tmp_rx_buf;
+	struct ethhdr *eth_head;
 
 	/* log if needed */
 	D1("Rx %d bytes", count);

@@ -1027,8 +1021,8 @@ static void packetizeRx(struct hso_net *
 static void read_bulk_callback(struct urb *urb)
 {
 	struct hso_net *odev = urb->context;
-	struct net_device *net = NULL;
-	int result = 0;
+	struct net_device *net;
+	int result;
 	int status = urb->status;
 
 	/* is al ok?  (Filip: Who's Al ?) */

@@ -1248,7 +1242,7 @@ static int hso_serial_write(struct tty_s
 			    int count)
 {
 	struct hso_serial *serial = get_serial_by_tty(tty);
-	int space = 0, tx_bytes = 0;
+	int space, tx_bytes;
 	unsigned long flags;
 
 	/* sanity check */

@@ -1281,7 +1275,7 @@ out:
 static int hso_serial_write_room(struct tty_struct *tty)
 {
 	struct hso_serial *serial = get_serial_by_tty(tty);
-	int room = 0;
+	int room;
 	unsigned long flags;
 
 	spin_lock_irqsave(&serial->serial_lock, flags);

@@ -1316,7 +1310,7 @@ static void hso_serial_set_termios(struc
 static int hso_serial_chars_in_buffer(struct tty_struct *tty)
 {
 	struct hso_serial *serial = get_serial_by_tty(tty);
-	int chars = 0;
+	int chars;
 	unsigned long flags;
 
 	/* sanity check */

@@ -1332,7 +1326,7 @@ static int hso_serial_chars_in_buffer(st
 
 static int hso_serial_tiocmget(struct tty_struct *tty, struct file *file)
 {
-	unsigned int value = 0;
+	unsigned int value;
 	struct hso_serial *serial = get_serial_by_tty(tty);
 	unsigned long flags;
 

@@ -1392,7 +1386,7 @@ static int hso_serial_tiocmset(struct tt
 /* starts a transmit */
 static void hso_kick_transmit(struct hso_serial *serial)
 {
-	u8 *temp = NULL;
+	u8 *temp;
 	unsigned long flags;
 	int res;
 

@@ -1430,8 +1424,8 @@ static int mux_device_request(struct hso
 			      struct usb_ctrlrequest *ctrl_req,
 			      u8 *ctrl_urb_data, u32 size)
 {
-	int result = 0;
-	int pipe = -1;
+	int result;
+	int pipe;
 
 	/* Sanity check */
 	if (!serial || !ctrl_urb || !ctrl_req) {

@@ -1512,10 +1506,10 @@ static int hso_mux_serial_read(struct hs
 static void intr_callback(struct urb *urb)
 {
 	struct hso_shared_int *shared_int = urb->context;
-	struct hso_serial *serial = NULL;
-	unsigned char *port_req = NULL;
+	struct hso_serial *serial;
+	unsigned char *port_req;
 	int status = urb->status;
-	int i = 0;
+	int i;
 
 	usb_mark_last_busy(urb->dev);
 

@@ -1602,7 +1596,7 @@ static void hso_std_serial_write_bulk_ca
 static int hso_std_serial_write_data(struct hso_serial *serial)
 {
 	int count = serial->tx_data_count;
-	int result = 0;
+	int result;
 
 	usb_fill_bulk_urb(serial->tx_urb,
 			  serial->parent->usb,

@@ -1626,7 +1620,7 @@ static int hso_std_serial_write_data(str
 static void ctrl_callback(struct urb *urb)
 {
 	struct hso_serial *serial = urb->context;
-	struct usb_ctrlrequest *req = NULL;
+	struct usb_ctrlrequest *req;
 	int status = urb->status;
 
 	/* sanity check */

@@ -1692,7 +1686,7 @@ static void put_rxbuf_data(struct urb *u
 static void hso_std_serial_read_bulk_callback(struct urb *urb)
 {
 	struct hso_serial *serial = urb->context;
-	int result = 0;
+	int result;
 	int status = urb->status;
 
 	/* sanity check */

@@ -1758,7 +1752,7 @@ static void hso_std_serial_read_bulk_cal
 
 static void hso_log_port(struct hso_device *hso_dev)
 {
-	char *port_type = NULL;
+	char *port_type;
 	char port_dev[20];
 
 	switch (hso_dev->port_spec & HSO_PORT_MASK) {

@@ -1958,7 +1952,7 @@ static void hso_serial_common_free(struc
 static int hso_serial_common_create(struct hso_serial *serial, int num_urbs,
 				    int rx_size, int tx_size)
 {
-	struct device *dev = NULL;
+	struct device *dev;
 	int minor;
 	int i;
 

@@ -2107,8 +2101,6 @@ static void hso_net_init(struct net_devi
 {
 	struct hso_net *hso_net = netdev_priv(net);
 
-	/* initialize to zero */
-	memset(hso_net, 0, sizeof(*hso_net));
 	D1("sizeof hso_net is %d", (int)sizeof(*hso_net));
 
 	/* most of the setup is done by standard function */

@@ -2118,7 +2110,6 @@ static void hso_net_init(struct net_devi
 	net->open = hso_net_open;
 	net->stop = hso_net_close;
 	net->hard_start_xmit = hso_net_start_xmit;
-	net->get_stats = hso_net_get_stats;
 	net->tx_timeout = hso_net_tx_timeout;
 	net->watchdog_timeo = HSO_NET_TX_TIMEOUT;
 	net->flags |= IFF_NOARP;

@@ -2137,8 +2128,7 @@ static void set_ethernet_addr(struct hso
 {
 	unsigned char mac_addr[6];
 	unsigned char dummy_mac[6];
-	int i = 0;
-	int len = 0;
+	int i, len;
 	unsigned char buffer[13];
 
 	/* we can't fail, therefor we use a random macaddress */

@@ -2197,9 +2187,9 @@ static int add_net_device(struct hso_dev
 static struct hso_device *hso_create_net_device(struct usb_interface *interface)
 {
 	int result, i;
-	struct net_device *net = NULL;
-	struct hso_net *hso_net = NULL;
-	struct hso_device *hso_dev = NULL;
+	struct net_device *net;
+	struct hso_net *hso_net;
+	struct hso_device *hso_dev;
 
 	hso_dev = hso_create_device(interface, HSO_INTF_MUX | HSO_PORT_NETWORK);
 	if (!hso_dev)

@@ -2319,8 +2309,8 @@ static void hso_free_serial_device(struc
 static struct hso_device *hso_create_bulk_serial_device(
 			struct usb_interface *interface, int port)
 {
-	struct hso_device *hso_dev = NULL;
-	struct hso_serial *serial = NULL;
+	struct hso_device *hso_dev;
+	struct hso_serial *serial;
 	char device_name[20];
 	int num_urbs;
 

@@ -2391,12 +2381,12 @@ struct hso_device *hso_create_mux_serial
 						int port,
 						struct hso_shared_int *mux)
 {
-	struct hso_device *hso_dev = NULL;
-	struct hso_serial *serial = NULL;
+	struct hso_device *hso_dev;
+	struct hso_serial *serial;
 	char device_name[20];
-	int port_spec = 0;
+	int port_spec;
 
-	port_spec |= HSO_INTF_MUX;
+	port_spec = HSO_INTF_MUX;
 	port_spec &= ~HSO_PORT_MASK;
 
 	port_spec |= hso_mux_to_port(port);

@@ -2506,7 +2496,7 @@ static int hso_get_config_data(struct us
 	struct usb_device *usbdev = interface_to_usbdev(interface);
 	u8 config_data[17];
 	u32 if_num = interface->altsetting->desc.bInterfaceNumber;
-	s32 result = 0;
+	s32 result;
 
 	if (usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
 			    0x86, 0xC0, 0, 0, config_data, 17,

@@ -2588,8 +2578,8 @@ static int hso_probe(struct usb_interfac
 	int mux, i, if_num, port_spec;
 	unsigned char port_mask;
 	char *rfkn;
-	struct hso_device *hso_dev = NULL;
-	struct hso_shared_int *shared_int = NULL;
+	struct hso_device *hso_dev;
+	struct hso_shared_int *shared_int;
 	struct hso_device *tmp_dev = NULL;
 
 	if_num = interface->altsetting->desc.bInterfaceNumber;

@@ -2760,7 +2750,7 @@ static int hso_put_activity(struct hso_d
 /* called by kernel when we need to suspend device */
 static int hso_suspend(struct usb_interface *iface, pm_message_t message)
 {
-	int i, result = 0;
+	int i, result;
 
 	/* Stop all serial ports */
 	for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) {

@@ -2872,7 +2862,7 @@ static struct usb_endpoint_descriptor *h
 {
 	int i;
 	struct usb_host_interface *iface = intf->cur_altsetting;
-	struct usb_endpoint_descriptor *endp = NULL;
+	struct usb_endpoint_descriptor *endp;
 
 	for (i = 0; i < iface->desc.bNumEndpoints; i++) {
 		endp = &iface->endpoint[i].desc;

@@ -2951,8 +2941,8 @@ static struct usb_driver hso_driver = {
 
 static int __init hso_init(void)
 {
-	int i = 0;
-	int result = 0;
+	int i;
+	int result;
 
 	/* put it in the log */
 	printk(KERN_INFO "hso: %s\n", version);

A message that is not in the archive (sent off-list or not yet received): [email protected]

#23 lore raw

Incremental patch.
- converted network interface from ethernet to PtP
- some includes cleanup
- fixed false gcc warning about using uninitialized variable. Introduced by previous patch.

Signed-of-by Paulius Zaleckas <[email protected]>
drivers/net/usb/hso.c renamed from drivers/net/usb/hso.c.orig +10 -97
@@ -47,8 +47,6 @@
 #include <linux/init.h>
 #include <linux/delay.h>
 #include <linux/netdevice.h>
-#include <linux/inetdevice.h>
-#include <linux/etherdevice.h>
 #include <linux/module.h>
 #include <linux/ethtool.h>
 #include <linux/usb.h>

@@ -171,7 +169,6 @@ struct hso_net {
 	unsigned short rx_buf_size;
 	unsigned short rx_buf_missing;
 	struct iphdr rx_ip_hdr;
-	struct ethhdr dummy_eth_head;
 
 	unsigned long flags;
 };

@@ -294,19 +291,6 @@ static int hso_get_activity(struct hso_d
 /* Helping functions                                                         */
 /*****************************************************************************/
 
-/* convert a character representing a hex value to a number */
-static unsigned char hex2dec(unsigned char digit)
-{
-	if ((digit >= '0') && (digit <= '9'))
-		return (digit - '0');
-	/* Map all characters to 0-15 */
-	if ((digit >= 'a') && (digit <= 'z'))
-		return (digit - 'a' + 10) % 16;
-	if ((digit >= 'A') && (digit <= 'Z'))
-		return (digit - 'A' + 10) % 16;
-	return 0;
-}
-
 /* #define DEBUG */
 
 #define dev2net(x) (x->port_data.dev_net)

@@ -790,8 +774,6 @@ static int hso_net_start_xmit(struct sk_
 		return 0;
 	}
 
-	/* fetch the packet */
-	skb_pull(skb, ETH_HLEN);
 	/* log if asked */
 	DUMP1(skb->data, skb->len);
 	/* Copy it from kernel memory to OUR memory */

@@ -870,7 +852,6 @@ static void packetizeRx(struct hso_net *
 	unsigned short buffer_offset = 0;
 	unsigned short frame_len;
 	unsigned char *tmp_rx_buf;
-	struct ethhdr *eth_head;
 
 	/* log if needed */
 	D1("Rx %d bytes", count);

@@ -909,9 +890,7 @@ static void packetizeRx(struct hso_net *
 					continue;
 				}
 				/* Allocate an sk_buff */
-				odev->skb_rx_buf = dev_alloc_skb(
-					frame_len + 2 +
-					odev->net->hard_header_len);
+				odev->skb_rx_buf = dev_alloc_skb(frame_len);
 				if (!odev->skb_rx_buf) {
 					/* We got no receive buffer. */
 					D1("could not allocate memory");

@@ -921,11 +900,6 @@ static void packetizeRx(struct hso_net *
 				/* Here's where it came from */
 				odev->skb_rx_buf->dev = odev->net;
 
-				/* Make some headroom: standard alignment + the
-				 * ethernet header. */
-				skb_reserve(odev->skb_rx_buf,
-					    2 + odev->net->hard_header_len);
-
 				/* Copy what we got so far. make room for iphdr
 				 * after tail. */
 				tmp_rx_buf =

@@ -935,9 +909,7 @@ static void packetizeRx(struct hso_net *
 				       sizeof(struct iphdr));
 
 				/* ETH_HLEN */
-				odev->rx_buf_size =
-				    odev->net->hard_header_len +
-				    sizeof(struct iphdr);
+				odev->rx_buf_size = sizeof(struct iphdr);
 
 				/* Filip actually use .tot_len */
 				odev->rx_buf_missing =

@@ -962,22 +934,15 @@ static void packetizeRx(struct hso_net *
 			odev->rx_buf_size += temp_bytes;
 			if (!odev->rx_buf_missing) {
 				/* Packet is complete. Inject into stack. */
-				/* Add fake ethernet header. */
-				eth_head = (struct ethhdr *)skb_push(odev->
-							      skb_rx_buf,
-							      odev->
-							      net->
-							      hard_header_len);
-				memcpy(eth_head, &odev->dummy_eth_head,
-				       sizeof(struct ethhdr));
-
-				/* Not sure here either */
-				odev->skb_rx_buf->protocol = eth_type_trans(
-							odev->skb_rx_buf,
-							odev->net);
+				/* We have IP packet here */
+				odev->skb_rx_buf->protocol =
+						__constant_htons(ETH_P_IP);
 				/* don't check it */
 				odev->skb_rx_buf->ip_summed =
 					CHECKSUM_UNNECESSARY;
+
+				skb_reset_mac_header(odev->skb_rx_buf);
+
 				/* Ship it off to the kernel */
 				netif_rx(odev->skb_rx_buf);
 				/* No longer our buffer. */

@@ -986,8 +951,6 @@ static void packetizeRx(struct hso_net *
 				/* update out statistics */
 				odev->net->stats.rx_packets++;
 
-				/* Hmmm, wonder if we have received the IP len
-				 * or the ETH len. */
 				odev->net->stats.rx_bytes += odev->rx_buf_size;
 
 				odev->rx_buf_size = 0;

@@ -2103,70 +2066,22 @@ static void hso_net_init(struct net_devi
 
 	D1("sizeof hso_net is %d", (int)sizeof(*hso_net));
 
-	/* most of the setup is done by standard function */
-	ether_setup(net);
-
 	/* fill in the other fields */
 	net->open = hso_net_open;
 	net->stop = hso_net_close;
 	net->hard_start_xmit = hso_net_start_xmit;
 	net->tx_timeout = hso_net_tx_timeout;
 	net->watchdog_timeo = HSO_NET_TX_TIMEOUT;
-	net->flags |= IFF_NOARP;
+	net->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
+	net->type = ARPHRD_NONE;
 	net->mtu = DEFAULT_MTU - 14;
 	net->tx_queue_len = 10;
 	SET_ETHTOOL_OPS(net, &ops);
-	hso_net->skb_rx_buf = NULL;
-	hso_net->rx_parse_state = WAIT_IP;
 
 	/* and initialize the semaphore */
 	spin_lock_init(&hso_net->net_lock);
 }
 
-/* setting the mac-address of the newly created iface */
-static void set_ethernet_addr(struct hso_net *odev)
-{
-	unsigned char mac_addr[6];
-	unsigned char dummy_mac[6];
-	int i, len;
-	unsigned char buffer[13];
-
-	/* we can't fail, therefor we use a random macaddress */
-	random_ether_addr(mac_addr);
-	dummy_mac[0] = 0xFA;
-	dummy_mac[1] = mac_addr[1];
-	dummy_mac[2] = mac_addr[2];
-	dummy_mac[3] = mac_addr[3];
-	dummy_mac[4] = mac_addr[4];
-	dummy_mac[5] = mac_addr[5];
-
-	/* but we like consistency, that's why we try to use the serial number
-	 * as a macaddress if available */
-	len = usb_string(odev->parent->usb,
-			 odev->parent->usb->descriptor.iSerialNumber,
-			 buffer, 13);
-	if (len != 12 && strcmp("Serial", buffer)) {
-		/* Fill in the mac_addr */
-		for (i = 0; i < 6; i++) {
-			mac_addr[i] = (hex2dec(buffer[2 * i]) << 4) +
-				       hex2dec(buffer[2 * i + 1]);
-		}
-		mac_addr[0] &= 0xfe;	/* clear multicast bit */
-		mac_addr[0] |= 0x02;	/* set local assignment bit (IEEE802) */
-	} else {
-		dev_err(&odev->parent->usb->dev, "Attempting to get MAC "
-			"address failed: using random\n");
-	}
-
-	/* Now copy it over to our network device structure */
-	memcpy(odev->net->dev_addr, mac_addr, sizeof(mac_addr));
-
-	/* Create the default fake ethernet header. */
-	memcpy(odev->dummy_eth_head.h_dest, mac_addr, ETH_ALEN);
-	memcpy(odev->dummy_eth_head.h_source, dummy_mac, ETH_ALEN);
-	odev->dummy_eth_head.h_proto = htons(ETH_P_IP);
-}
-
 /* Adds a network device in the network device table */
 static int add_net_device(struct hso_device *hso_dev)
 {

@@ -2255,9 +2170,6 @@ static struct hso_device *hso_create_net
 		goto exit;
 	}
 
-	/* and don't forget the MAC address. */
-	set_ethernet_addr(hso_net);

A message that is not in the archive (sent off-list or not yet received): [email protected]

#25 lore raw

Incremental patch.
- don't reset rx_packets stats when telling device "up".

TODO:
- figure out what should be MTU value. Currently it is set to 1500-14. I guess that 14 here means eth header size, but this has no logical explanation...
- do we really need ethtool support? It always reports that link is up... The only useful information is driver name, version and USB path... Is it worth it?

drivers/net/usb/hso.c renamed from drivers/net/usb/hso.c.orig +1 -2
@@ -693,8 +693,7 @@ static int hso_net_open(struct net_devic
 		dev_err(&net->dev, "No net device !\n");
 		return -ENODEV;
 	}
-	/* reset read counter. */
-	odev->net->stats.rx_packets = 0;
+
 	odev->skb_tx_buf = NULL;
 
 	/* setup environment */
#26 lore raw

Am Freitag, 25. April 2008 14:31:47 schrieb Paulius Zaleckas:

Quoted 3 lines

- do we really need ethtool support? It always reports that link is up... The only useful information is driver name, version and USB path... Is it worth it?

Yes. NetworkManager does strange things to devices without ethtool support. NetworkManager is important.

Regards
Oliver

A message that is not in the archive (sent off-list or not yet received): 200804251503.31240.oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org

#28 lore raw
Quoted 7 lines · Oliver Neukum

Am Freitag, 25. April 2008 14:31:47 schrieb Paulius Zaleckas:

Quoted 3 lines

- do we really need ethtool support? It always reports that ?? ??link is up... The only useful information is driver name, ?? ??version and USB path... Is it worth it?

Yes. NetworkManager does strange things to devices without ethtool support. NetworkManager is important.

So we hack around broken userspace in the kernel now? What about using your energy to fix up NetworkManager (yes, I know it's crap code, but life is hard sometimes)

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

#30 lore raw

Incremental patch.
- simplify net stats even more

drivers/net/usb/hso.c renamed from drivers/net/usb/hso.c.orig +4 -4
@@ -796,11 +796,11 @@ static int hso_net_start_xmit(struct sk_
 	if (result) {
 		dev_warn(&odev->parent->interface->dev,
 			"failed mux_bulk_tx_urb %d", result);
-		odev->net->stats.tx_errors++;
+		net->stats.tx_errors++;
 		netif_start_queue(net);
 	} else {
-		odev->net->stats.tx_packets++;
-		odev->net->stats.tx_bytes += skb->len;
+		net->stats.tx_packets++;
+		net->stats.tx_bytes += skb->len;
 		/* And tell the kernel when the last transmit started. */
 		net->trans_start = jiffies;
 	}

@@ -840,7 +840,7 @@ static void hso_net_tx_timeout(struct ne
 		usb_unlink_urb(odev->mux_bulk_tx_urb);
 
 	/* Update statistics */
-	odev->net->stats.tx_errors++;
+	net->stats.tx_errors++;
 }
 
 /* make a real packet from the received USB buffer */

A message that is not in the archive (sent off-list or not yet received): [email protected]

#32 lore raw

Incremental patch.
- fix bug with freeing locked mutex:

Quoted 10 lines

=========================
[ BUG: held lock freed! ]
-------------------------
modprobe/3829 is freeing memory ffff81002acb5948-ffff81002acb5a07, with a lock still held there!
(&mux->shared_int_lock){--..}, at: [<ffffffff88303234>]

hso_serial_ref_free+0x7a/0xce [hso]
1 lock held by modprobe/3829:
 #0:  (&mux->shared_int_lock){--..}, at: [<ffffffff88303234>]
hso_serial_ref_free+0x7a/0xce [hso]
drivers/net/usb/hso.c renamed from drivers/net/usb/hso.c.orig +3 -1
@@ -2210,7 +2210,8 @@ static void hso_free_serial_device(struc
 		mutex_lock(&serial->shared_int->shared_int_lock);
 		if (--serial->shared_int->ref_count == 0)
 			hso_free_shared_int(serial->shared_int);
-		mutex_unlock(&serial->shared_int->shared_int_lock);
+		else
+			mutex_unlock(&serial->shared_int->shared_int_lock);
 	}
 	kfree(serial);
 	hso_free_device(hso_dev);

@@ -2360,6 +2361,7 @@ static void hso_free_shared_int(struct h
 {
 	usb_free_urb(mux->shared_intr_urb);
 	kfree(mux->shared_intr_buf);
+	mutex_unlock(&mux->shared_int_lock);
 	kfree(mux);
 }

A message that is not in the archive (sent off-list or not yet received): [email protected]

#34 lore raw

Incremental patch.
- Oliver patch for rfkill crash (introduced rfkill/mem leak)
- Create rfkill only for network device (fixed rfkill/mem leak)
- Make driver work even if rfkill initialization fails

Greg: What do you think about unified patch?

drivers/net/usb/hso.c renamed from drivers/net/usb/hso.c.orig +55 -48
@@ -151,6 +151,7 @@ struct hso_shared_int {
 struct hso_net {
 	struct hso_device *parent;
 	struct net_device *net;
+	struct rfkill *rfkill;
 
 	struct usb_endpoint_descriptor *in_endp;
 	struct usb_endpoint_descriptor *out_endp;

@@ -227,7 +228,6 @@ struct hso_device {
 
 	struct usb_device *usb;
 	struct usb_interface *interface;
-	struct rfkill *rfkill;
 
 	struct device *dev;
 	struct kref ref;

@@ -2097,6 +2097,58 @@ static int add_net_device(struct hso_dev
 	return 0;
 }
 
+static int hso_radio_toggle(void *data, enum rfkill_state state)
+{
+	struct hso_device *hso_dev = data;
+	int enabled = (state == RFKILL_STATE_ON);
+	int rv;
+
+	mutex_lock(&hso_dev->mutex);
+	if (hso_dev->usb_gone)
+		rv = 0;
+	else
+		rv = usb_control_msg(hso_dev->usb, usb_rcvctrlpipe(hso_dev->usb, 0),
+				       enabled ? 0x82 : 0x81, 0x40, 0, 0, NULL, 0,
+				       USB_CTRL_SET_TIMEOUT);
+	mutex_unlock(&hso_dev->mutex);
+	return rv;
+}
+
+/* Creates and sets up everything for rfkill */
+static void hso_create_rfkill(struct hso_device *hso_dev,
+			     struct usb_interface *interface)
+{
+	struct hso_net *hso_net = dev2net(hso_dev);
+	struct device *dev = hso_dev->dev;
+	char *rfkn;
+
+	hso_net->rfkill = rfkill_allocate(&interface_to_usbdev(interface)->dev,
+				 RFKILL_TYPE_WLAN);
+	if (!hso_net->rfkill) {
+		dev_err(dev, "%s - Out of memory", __func__);
+		return;
+	}
+	rfkn = kzalloc(20, GFP_KERNEL);
+	if (!rfkn) {
+		rfkill_free(hso_net->rfkill);
+		dev_err(dev, "%s - Out of memory", __func__);
+		return;
+	}
+	snprintf(rfkn, 20, "hso-%d",
+		 interface->altsetting->desc.bInterfaceNumber);
+	hso_net->rfkill->name = rfkn;
+	hso_net->rfkill->state = RFKILL_STATE_ON;
+	hso_net->rfkill->data = hso_dev;
+	hso_net->rfkill->toggle_radio = hso_radio_toggle;
+	if (rfkill_register(hso_net->rfkill) < 0) {
+		kfree(rfkn);
+		hso_net->rfkill->name = NULL;
+		rfkill_free(hso_net->rfkill);
+		dev_err(dev, "%s - Failed to register rfkill", __func__);
+		return;
+	}
+}
+
 /* Creates our network device */
 static struct hso_device *hso_create_net_device(struct usb_interface *interface)
 {

@@ -2183,6 +2235,8 @@ static struct hso_device *hso_create_net
 
 	hso_log_port(hso_dev);
 
+	hso_create_rfkill(hso_dev, interface);
+
 	return hso_dev;
 exit:
 	hso_free_net_device(hso_dev);

@@ -2467,30 +2521,12 @@ static int hso_get_config_data(struct us
 	return result;
 }
 
-static int hso_radio_toggle(void *data, enum rfkill_state state)
-{
-	struct hso_device *hso_dev = data;
-	int enabled = (state == RFKILL_STATE_ON);
-	int rv;
-
-	mutex_lock(&hso_dev->mutex);
-	if (hso_dev->usb_gone)
-		rv = 0;
-	else
-		rv = usb_control_msg(hso_dev->usb, usb_rcvctrlpipe(hso_dev->usb, 0),
-				       enabled ? 0x82 : 0x81, 0x40, 0, 0, NULL, 0,
-				       USB_CTRL_SET_TIMEOUT);
-	mutex_unlock(&hso_dev->mutex);
-	return rv;
-}
-
 /* called once for each interface upon device insertion */
 static int hso_probe(struct usb_interface *interface,
 		     const struct usb_device_id *id)
 {
 	int mux, i, if_num, port_spec;
 	unsigned char port_mask;
-	char *rfkn;
 	struct hso_device *hso_dev = NULL;
 	struct hso_shared_int *shared_int;
 	struct hso_device *tmp_dev = NULL;

@@ -2563,27 +2599,6 @@ static int hso_probe(struct usb_interfac
 		goto exit;
 	}
 
-	hso_dev->rfkill = rfkill_allocate(&interface_to_usbdev(interface)->dev,
-					  RFKILL_TYPE_WLAN);
-	if (!hso_dev->rfkill)
-		goto exit;
-	rfkn = kzalloc(20, GFP_KERNEL);
-	if (!rfkn) {
-		rfkill_free(hso_dev->rfkill);
-		goto exit;
-	}
-	snprintf(rfkn, 20, "hso-%d", if_num);
-	hso_dev->rfkill->name = rfkn;
-	hso_dev->rfkill->state = RFKILL_STATE_ON;
-	hso_dev->rfkill->data = hso_dev;
-	hso_dev->rfkill->toggle_radio = hso_radio_toggle;
-	if (rfkill_register(hso_dev->rfkill) < 0) {
-		kfree(rfkn);
-		hso_dev->rfkill->name = NULL;
-		rfkill_free(hso_dev->rfkill);
-		goto exit;
-	}
-
 	usb_driver_claim_interface(&hso_driver, interface, hso_dev);
 
 	/* save our data pointer in this device */

@@ -2591,7 +2606,6 @@ static int hso_probe(struct usb_interfac
 
 	/* done */
 	return 0;
-
 exit:
 	hso_free_interface(interface);
 	return -ENODEV;

@@ -2600,16 +2614,8 @@ exit:
 /* device removed, cleaning up */
 static void hso_disconnect(struct usb_interface *interface)
 {
-	struct hso_device *hso_dev = usb_get_intfdata(interface);
-
 	hso_free_interface(interface);
 
-	if (hso_dev) {
-		cancel_work_sync(&hso_dev->async_put_intf);
-		cancel_work_sync(&hso_dev->async_get_intf);
-		rfkill_unregister(hso_dev->rfkill);
-	}
#35 lore raw
In reply to a message not in the archive

Here is the crash dump when I do "modprobe -r hso". I think I had no problems when I was using not debug version of kernel.

hso: unloaded
usbcore: deregistering interface driver hso

general protection fault: 0000 [1] SMP
CPU 1
Modules linked in: i915 drm autofs4 fuse sunrpc loop dm_multipath ipv6 parport_pc parport snd_hda_intel pcspkr i2c_i801 r8169 snd_seq_dummy e100 i2c_core mii iTCO_wdt snd_seq_oss hso(U) usblp snd_seq_midi_event iTCO_vendor_support rfkill snd_seq snd_seq_device snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd_page_alloc snd_hwdep snd soundcore button sg sr_mod cdrom dm_snapshot dm_zero dm_mirror dm_mod pata_acpi ata_generic ata_piix libata sd_mod scsi_mod ext3 jbd mbcache uhci_hcd ohci_hcd ehci_hcd
Pid: 2926, comm: modprobe Not tainted 2.6.24.4-64.fc8debug #1
RIP: 0010:[<ffffffff810559b6>]  [<ffffffff810559b6>] __lock_acquire+0xb6/0xce5
RSP: 0000:ffff810010497c88  EFLAGS: 00010046
RAX: 0000000000000002 RBX: 0000000000000046 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 6b6b6b6b6b6b6b80
RBP: 6b6b6b6b6b6b6b68 R08: 0000000000000002 R09: 0000000000000001
R10: ffffffff81046da4 R11: 0000000000000008 R12: ffff8100105b0000
R13: ffffffff88191780 R14: 00000000ffffffff R15: 6b6b6b6b6b6b6b80
FS:  00002aaaaaac76f0(0000) GS:ffff81003f0014b0(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 00000000008ca3f0 CR3: 000000001e917000 CR4: 00000000000006a0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process modprobe (pid: 2926, threadinfo ffff810010496000, task ffff8100105b0000)
Stack:  000000018149c2e8 0000000000000002 0000000000000000 ffffffff8149c2b8
 ffff81003d8a8920 0000000000000046 6b6b6b6b6b6b6b68 ffff81003d1a6890
 ffffffff88191780 00000000ffffffff 00000000ffffffff ffffffff81056a45
Call Trace:
 [<ffffffff81056a45>] lock_acquire+0x5a/0x73
 [<ffffffff81046da4>] __cancel_work_timer+0x51/0x1b5
 [<ffffffff812865c5>] _spin_lock_irq+0x2c/0x58
 [<ffffffff81046da4>] __cancel_work_timer+0x51/0x1b5
 [<ffffffff8105548e>] trace_hardirqs_on+0x12e/0x151
 [<ffffffff8818b317>] :hso:hso_serial_ref_free+0xc5/0xce
 [<ffffffff8105548e>] trace_hardirqs_on+0x12e/0x151
 [<ffffffff8818b252>] :hso:hso_serial_ref_free+0x0/0xce
 [<ffffffff81131c5d>] kref_put+0x6f/0x7a
 [<ffffffff8818b10b>] :hso:hso_disconnect+0x23/0x55
 [<ffffffff811d128b>] usb_unbind_interface+0x5c/0xb9
 [<ffffffff811b710d>] __device_release_driver+0x91/0xb3
 [<ffffffff811b7653>] driver_detach+0xd3/0x114
 [<ffffffff811b6be4>] bus_remove_driver+0x80/0xa3
 [<ffffffff811d0c1b>] usb_deregister+0x97/0xa4
 [<ffffffff8818d926>] :hso:hso_exit+0x2a/0x6a
 [<ffffffff8105ef9c>] sys_delete_module+0x18c/0x1bd
 [<ffffffff8100c053>] tracesys+0xd5/0xda


Code: 48 8b 6f 08 48 85 ed 0f 85 db 02 00 00 49 83 3f 00 75 03 4d
RIP  [<ffffffff810559b6>] __lock_acquire+0xb6/0xce5
 RSP <ffff810010497c88>
---[ end trace 8adbc80e43569c09 ]---
BUG: sleeping function called from invalid context at kernel/rwsem.c:21
in_atomic():0, irqs_disabled():1
INFO: lockdep is turned off.
irq event stamp: 5090
hardirqs last  enabled at (5089): [<ffffffff810a4773>] kfree+0xe1/0xec
hardirqs last disabled at (5090): [<ffffffff812865a8>] _spin_lock_irq+0xf/0x58
softirqs last  enabled at (4848): [<ffffffff8100d0cc>] call_softirq+0x1c/0x28
softirqs last disabled at (4835): [<ffffffff8100d0cc>] call_softirq+0x1c/0x28

Pid: 2926, comm: modprobe Tainted: G D 2.6.24.4-64.fc8debug #1

Call Trace:
 [<ffffffff8102f4ca>] __might_sleep+0xd5/0xdd
 [<ffffffff81285256>] down_read+0x1e/0x6c
 [<ffffffff810653bb>] acct_collect+0x42/0x18f
 [<ffffffff8103ad47>] do_exit+0x21e/0x7de
 [<ffffffff8100d6e6>] kernel_math_error+0x0/0x71
 [<ffffffff812869ad>] error_exit+0x0/0xa9
 [<ffffffff81046da4>] __cancel_work_timer+0x51/0x1b5
 [<ffffffff810559b6>] __lock_acquire+0xb6/0xce5
 [<ffffffff81056a45>] lock_acquire+0x5a/0x73
 [<ffffffff81046da4>] __cancel_work_timer+0x51/0x1b5
 [<ffffffff812865c5>] _spin_lock_irq+0x2c/0x58
 [<ffffffff81046da4>] __cancel_work_timer+0x51/0x1b5
 [<ffffffff8105548e>] trace_hardirqs_on+0x12e/0x151
 [<ffffffff8818b317>] :hso:hso_serial_ref_free+0xc5/0xce
 [<ffffffff8105548e>] trace_hardirqs_on+0x12e/0x151
 [<ffffffff8818b252>] :hso:hso_serial_ref_free+0x0/0xce
 [<ffffffff81131c5d>] kref_put+0x6f/0x7a
 [<ffffffff8818b10b>] :hso:hso_disconnect+0x23/0x55
 [<ffffffff811d128b>] usb_unbind_interface+0x5c/0xb9
 [<ffffffff811b710d>] __device_release_driver+0x91/0xb3
 [<ffffffff811b7653>] driver_detach+0xd3/0x114
 [<ffffffff811b6be4>] bus_remove_driver+0x80/0xa3
 [<ffffffff811d0c1b>] usb_deregister+0x97/0xa4
 [<ffffffff8818d926>] :hso:hso_exit+0x2a/0x6a
 [<ffffffff8105ef9c>] sys_delete_module+0x18c/0x1bd
 [<ffffffff8100c053>] tracesys+0xd5/0xda

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html