On Tue, 5 Mar 2002, Greg KH wrote:
> On Tue, Mar 05, 2002 at 10:40:59AM -0800, Greg KH wrote:
> >
> > Ah, that's it. include/net/irda/irda-usb.h was missed in the big "no
> > more static urb allocations" fixup. Give me a few minutes to change
> > this.
>
> Jean, does the patch below fix your problem?
>
[...]
> - urb = &self->speed_urb;
> + urb = self->speed_urb;
Hi Greg and Jean,
I haven't tried the patch but I'm sure the problem is more than this:
Besides the dynamic alloc enforcement there was the memflag change for
usb_submit_urb() as well. irda-usb calls this either with spinlock held or
from places which are either in process context or interrupt (resubmit on
rx-urb completion). Hence GFP_KERNEL isn't a good choice here.
Did some band-aid on it to address both issues:
- all embedded urb's (tx_urb, speed_urb, rx_urb[] pool) changed to
struct urb * in the control block. This is basically a modified version
of your patch, however the usb_alloc_urb()/usb_free_urb() are moved
from probe/disconnect to the netdev->open() and close() path
respectively. This is because I'm unsure whether there are disconnect
races: If the urb's are free'd on disconnect, are we sure netdev->xmit
wouldn't try to use them?
- changed memflags when calling usb_submit_urb() to either GFP_ATOMIC
unconditionally when inside spinlock or depending on
in_interrupt() otherwise.
With the patch below things seem to work for me on OHCI - at least
discovery is fine from both ends. Patch is meant for 2.5.5 final or
later. However, I had only 2.5.5-pre1 ready for fast application
hence it was tried this way - with the memflags in usb_alloc_urb()
backed out - AFAICS it's the only change thereafter.
Furthermore, an additional isa_virt_to_bus() update is required for
net/irda/irda_device.c to get things rolling for any recent 2.5.x.
Martin
-----------------------------------------
--- linux-2.5.5-pre1/include/net/irda/irda-usb.h Wed Nov 28 15:17:28 2001
+++ v2.5.5-pre1-md/include/net/irda/irda-usb.h Wed Mar 6 00:09:36 2002
@@ -139,10 +139,10 @@
wait_queue_head_t wait_q; /* for timeouts */
- struct urb rx_urb[IU_MAX_RX_URBS]; /* URBs used to receive data frames */
+ struct urb *rx_urb[IU_MAX_RX_URBS]; /* URBs used to receive data frames */
struct urb *idle_rx_urb; /* Pointer to idle URB in Rx path */
- struct urb tx_urb; /* URB used to send data frames */
- struct urb speed_urb; /* URB used to send speed commands */
+ struct urb *tx_urb; /* URB used to send data frames */
+ struct urb *speed_urb; /* URB used to send speed commands */
struct net_device *netdev; /* Yes! we are some kind of netdev. */
struct net_device_stats stats;
--- linux-2.5.5-pre1/drivers/net/irda/irda-usb.c Tue Feb 19 14:46:17 2002
+++ v2.5.5-pre1-md/drivers/net/irda/irda-usb.c Wed Mar 6 01:21:51 2002
@@ -255,7 +255,7 @@
self->new_speed, self->new_xbofs);
/* Grab the speed URB */
- urb = &self->speed_urb;
+ urb = self->speed_urb;
if (urb->status != 0) {
WARNING(__FUNCTION__ "(), URB still in use!\n");
return;
@@ -278,7 +278,7 @@
urb->transfer_flags = USB_QUEUE_BULK | USB_ASYNC_UNLINK;
urb->timeout = MSECS_TO_JIFFIES(100);
- if ((ret = usb_submit_urb(urb, GFP_KERNEL))) {
+ if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) { /* under spinlock! */
WARNING(__FUNCTION__ "(), failed Speed URB\n");
}
spin_unlock_irqrestore(&self->lock, flags);
@@ -317,7 +317,7 @@
urb->status = 0;
/* If it was the speed URB, allow the stack to send more packets */
- if(urb == &self->speed_urb) {
+ if(urb == self->speed_urb) {
netif_wake_queue(self->netdev);
}
}
@@ -329,7 +329,7 @@
static int irda_usb_hard_xmit(struct sk_buff *skb, struct net_device *netdev)
{
struct irda_usb_cb *self = netdev->priv;
- struct urb *urb = &self->tx_urb;
+ struct urb *urb = self->tx_urb;
unsigned long flags;
s32 speed;
s16 xbofs;
@@ -451,7 +451,7 @@
}
/* Ask USB to send the packet */
- if ((res = usb_submit_urb(urb, GFP_KERNEL))) {
+ if ((res = usb_submit_urb(urb, GFP_ATOMIC))) { /* under spinlock */
WARNING(__FUNCTION__ "(), failed Tx URB\n");
self->stats.tx_errors++;
/* Let USB recover : We will catch that in the watchdog */
@@ -546,7 +546,7 @@
}
/* Check speed URB */
- urb = &(self->speed_urb);
+ urb = self->speed_urb;
if (urb->status != 0) {
IRDA_DEBUG(0, "%s: Speed change timed out, urb->status=%d,
urb->transfer_flags=0x%04X\n", netdev->name, urb->status, urb->transfer_flags);
@@ -571,7 +571,7 @@
}
/* Check Tx URB */
- urb = &(self->tx_urb);
+ urb = self->tx_urb;
if (urb->status != 0) {
struct sk_buff *skb = urb->context;
@@ -730,7 +730,8 @@
urb->status = 0;
urb->next = NULL; /* Don't auto resubmit URBs */
- ret = usb_submit_urb(urb, GFP_KERNEL);
+ /* called both from netdev->open _and_ urb_complete isr!*/
+ ret = usb_submit_urb(urb, (in_interrupt()) ? GFP_ATOMIC : GFP_KERNEL);
if (ret) {
/* If this ever happen, we are in deep s***.
* Basically, the Rx path will stop... */
@@ -915,7 +916,7 @@
{
struct irda_usb_cb *self;
char hwname[16];
- int i;
+ int i, j;
IRDA_DEBUG(1, __FUNCTION__ "()\n");
@@ -936,6 +937,34 @@
self->new_speed = -1;
self->new_xbofs = -1;
+ /* allocate our urbs */
+ self->speed_urb = usb_alloc_urb(0, GFP_KERNEL);
+ if (!self->speed_urb) {
+ WARNING("%s, speed urb alloc failed\n", __FUNCTION__);
+ return -ENOMEM;
+ }
+ self->tx_urb = usb_alloc_urb(0, GFP_KERNEL);
+ if (!self->tx_urb) {
+ WARNING("%s, tx urb alloc failed\n", __FUNCTION__);
+ usb_free_urb(self->speed_urb);
+ self->speed_urb = NULL;
+ return -ENOMEM;
+ }
+ for (i = 0; i < IU_MAX_RX_URBS; i++) { /* including idle_urb! */
+ self->rx_urb[i] = usb_alloc_urb(0, GFP_KERNEL);
+ if (!self->rx_urb[i]) {
+ WARNING("%s, rx urb pool alloc failed\n", __FUNCTION__);
+ for (j = 0; j < i; j++) {
+ usb_free_urb(self->rx_urb[j]);
+ self->rx_urb[j] = NULL;
+ }
+ usb_free_urb(self->speed_urb);
+ usb_free_urb(self->tx_urb);
+ self->speed_urb = self->tx_urb = NULL;
+ return -ENOMEM;
+ }
+ }
+
/* To do *before* submitting Rx urbs and starting net Tx queue
* Jean II */
self->netopen = 1;
@@ -955,9 +984,9 @@
/* Now that we can pass data to IrLAP, allow the USB layer
* to send us some data... */
for (i = 0; i < IU_MAX_ACTIVE_RX_URBS; i++)
- irda_usb_submit(self, NULL, &(self->rx_urb[i]));
+ irda_usb_submit(self, NULL, self->rx_urb[i]);
/* Note : we submit all the Rx URB except for one - Jean II */
- self->idle_rx_urb = &(self->rx_urb[IU_MAX_ACTIVE_RX_URBS]);
+ self->idle_rx_urb = self->rx_urb[IU_MAX_ACTIVE_RX_URBS];
self->idle_rx_urb->context = NULL;
/* Ready to play !!! */
@@ -992,7 +1021,7 @@
/* Deallocate all the Rx path buffers (URBs and skb) */
for (i = 0; i < IU_MAX_RX_URBS; i++) {
- struct urb *urb = &(self->rx_urb[i]);
+ struct urb *urb = self->rx_urb[i];
struct sk_buff *skb = (struct sk_buff *) urb->context;
/* Cancel the receive command */
usb_unlink_urb(urb);
@@ -1003,14 +1032,24 @@
}
}
/* Cancel Tx and speed URB */
- usb_unlink_urb(&(self->tx_urb));
- usb_unlink_urb(&(self->speed_urb));
+ usb_unlink_urb(self->tx_urb);
+ usb_unlink_urb(self->speed_urb);
/* Stop and remove instance of IrLAP */
if (self->irlap)
irlap_close(self->irlap);
self->irlap = NULL;
+ /* free all the urbs - above are all synchrous unlinks! */
+
+ for (i = 0; i < IU_MAX_RX_URBS; i++) { /* including idle_urb! */
+ usb_free_urb(self->rx_urb[i]);
+ self->rx_urb[i] = NULL;
+ }
+ usb_free_urb(self->tx_urb);
+ usb_free_urb(self->speed_urb);
+ self->tx_urb = self->speed_urb = NULL;
+
MOD_DEC_USE_COUNT;
return 0;
@@ -1501,10 +1540,10 @@
netif_stop_queue(self->netdev);
/* Stop all the receive URBs */
for (i = 0; i < IU_MAX_RX_URBS; i++)
- usb_unlink_urb(&(self->rx_urb[i]));
+ usb_unlink_urb(self->rx_urb[i]);
/* Cancel Tx and speed URB */
- usb_unlink_urb(&(self->tx_urb));
- usb_unlink_urb(&(self->speed_urb));
+ usb_unlink_urb(self->tx_urb);
+ usb_unlink_urb(self->speed_urb);
IRDA_DEBUG(0, __FUNCTION__ "(), postponing disconnect, network is
still active...\n");
/* better not do anything just yet, usb_irda_cleanup()
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
