clear_halt_work
Roman Kagan
rkagan at mail.ru
Thu May 5 16:25:00 EDT 2005
On Thu, May 05, 2005 at 10:41:30AM +0200, Duncan Sands wrote:
> Hi Roman, the only thing I'm still unhappy about is the
> clear-halt stuff (since you are supposed to make sure no
> urbs are in flight when clearing the halt). How about we
> just remove it for the moment.
I'm fine with removing it for now; it's fairly local and should be easy
to restore when we figure how to do it right.
Meanwhile, I'd like to propose one more change (the last one, I'll shut
up until you push the stuff to Greg, promise :)
As per David Brownell's comment, usb core only uses urb_list field in
struct urb when it owns that urb, that is, since the call to
usb_submit_urb() till the entry into the completion routine. Outside of
this scope, drivers are free to use it for whatever they want. Thus we
can get rid of struct usbatm_transceiver, and use struct urb directly
instead.
The following patch does this. In addition, I've simplified a bit the
allocation of the array of urbs (formerly transceivers), merging it into
the allocation of struct usbatm_data. (Also the bits related to
-EAGAIN has sneaked in, if you still disagree with those feel free to
strip them away.)
The patch is briefly tested and doesn't appear to lead to any failures.
BTW shouldn't we set lower limits on the number of urbs and the buffer
sizes? I'm pretty certain the driver won't work with no urbs and/or
with a zero length buffer. OTOH upper limits are arbitrary and probably
superfluous: we seem to handle out-of-memory conditions gracefully.
Cheers,
Roman.
Index: usbatm.h
===================================================================
RCS file: /home/cvs/usbatm/usbatm.h,v
retrieving revision 1.16
diff -u -p -r1.16 usbatm.h
--- usbatm.h 3 May 2005 14:18:05 -0000 1.16
+++ usbatm.h 5 May 2005 19:36:55 -0000
@@ -137,12 +137,6 @@ struct usbatm_channel {
struct usbatm_data *usbatm;
};
-struct usbatm_transceiver {
- struct list_head list;
- struct urb *urb;
- struct usbatm_channel *channel;
-};
-
/* main driver data */
struct usbatm_data {
@@ -178,13 +172,13 @@ struct usbatm_data {
/* ATM device */
struct list_head vcc_list;
- struct usbatm_transceiver *transceivers;
-
struct usbatm_channel rx_channel;
struct usbatm_channel tx_channel;
struct sk_buff_head sndqueue;
struct sk_buff *current_skb; /* being emptied */
+
+ struct urb *urbs[0];
};
#endif /* _USBATM_H_ */
Index: usbatm.c
===================================================================
RCS file: /home/cvs/usbatm/usbatm.c,v
retrieving revision 1.38
diff -u -p -r1.38 usbatm.c
--- usbatm.c 5 May 2005 09:05:56 -0000 1.38
+++ usbatm.c 5 May 2005 19:36:56 -0000
@@ -203,13 +203,13 @@ static inline void usbatm_pop(struct atm
}
-/*******************
-** transceivers **
-*******************/
+/***********
+** urbs **
+************/
-static inline struct usbatm_transceiver *usbatm_pop_transceiver(struct usbatm_channel *channel)
+static inline struct urb *usbatm_pop_urb(struct usbatm_channel *channel)
{
- struct usbatm_transceiver *trx;
+ struct urb *urb;
spin_lock_irq(&channel->lock);
if (list_empty(&channel->list)) {
@@ -217,33 +217,32 @@ static inline struct usbatm_transceiver
return NULL;
}
- trx = list_entry(channel->list.next, struct usbatm_transceiver, list);
- list_del(&trx->list);
+ urb = list_entry(channel->list.next, struct urb, urb_list);
+ list_del(&urb->urb_list);
spin_unlock_irq(&channel->lock);
- return trx;
+ return urb;
}
-static inline int usbatm_submit_transceiver(struct usbatm_transceiver *trx)
+static inline int usbatm_submit_urb(struct urb *urb)
{
- struct usbatm_channel *channel = trx->channel;
+ struct usbatm_channel *channel = urb->context;
int ret;
- vdbg("%s: submitting urb 0x%p, trx 0x%p, size %u",
- __func__, trx->urb, trx, trx->urb->transfer_buffer_length);
+ vdbg("%s: submitting urb 0x%p, size %u",
+ __func__, urb, urb->transfer_buffer_length);
- ret = usb_submit_urb(trx->urb, GFP_ATOMIC);
+ ret = usb_submit_urb(urb, GFP_ATOMIC);
if (ret) {
- atm_dbg(channel->usbatm, "%s: trx 0x%p urb 0x%p submission failed (%d)!\n",
- __func__, trx, trx->urb, ret);
-
- trx->urb->status = ret;
+ atm_dbg(channel->usbatm, "%s: urb 0x%p submission failed (%d)!\n",
+ __func__, urb, ret);
/* consider all errors transient and return the buffer back to the queue */
+ urb->status = -EAGAIN;
spin_lock_irq(&channel->lock);
/* must add to the front when sending; doesn't matter when receiving */
- list_add(&trx->list, &channel->list);
+ list_add(&urb->urb_list, &channel->list);
spin_unlock_irq(&channel->lock);
@@ -256,18 +255,17 @@ static inline int usbatm_submit_transcei
static void usbatm_complete(struct urb *urb, struct pt_regs *regs)
{
- struct usbatm_transceiver *trx = urb->context;
- struct usbatm_channel *channel = trx->channel;
+ struct usbatm_channel *channel = urb->context;
unsigned long flags;
- vdbg("%s: urb 0x%p, status %d, actual_length %d, trx 0x%p",
- __func__, urb, urb->status, urb->actual_length, trx);
+ vdbg("%s: urb 0x%p, status %d, actual_length %d",
+ __func__, urb, urb->status, urb->actual_length);
/* usually in_interrupt(), but not always */
spin_lock_irqsave(&channel->lock, flags);
/* must add to the back when receiving; doesn't matter when sending */
- list_add_tail(&trx->list, &channel->list);
+ list_add_tail(&urb->urb_list, &channel->list);
spin_unlock_irqrestore(&channel->lock, flags);
@@ -485,11 +483,9 @@ static unsigned int usbatm_write_cells(s
static void usbatm_rx_process(unsigned long data)
{
struct usbatm_data *instance = (struct usbatm_data *)data;
- struct usbatm_transceiver *rx;
-
- while ((rx = usbatm_pop_transceiver(&instance->rx_channel))) {
- struct urb *urb = rx->urb;
+ struct urb *urb;
+ while ((urb = usbatm_pop_urb(&instance->rx_channel))) {
vdbg("%s: processing rx 0x%p", __func__, rx);
if (usb_pipeisoc(urb->pipe)) {
@@ -504,7 +500,7 @@ static void usbatm_rx_process(unsigned l
if (!urb->status)
usbatm_extract_cells(instance, urb->transfer_buffer, urb->actual_length);
- if (usbatm_submit_transceiver(rx))
+ if (usbatm_submit_urb(urb))
return;
}
}
@@ -518,7 +514,7 @@ static void usbatm_tx_process(unsigned l
{
struct usbatm_data *instance = (struct usbatm_data *)data;
struct sk_buff *skb = instance->current_skb;
- struct usbatm_transceiver *tx = NULL;
+ struct urb *urb = NULL;
const unsigned int buf_size = instance->tx_channel.buf_size;
unsigned int num_written = 0;
u8 *buffer = NULL;
@@ -527,20 +523,21 @@ static void usbatm_tx_process(unsigned l
skb = skb_dequeue(&instance->sndqueue);
while (skb) {
- if (!tx) {
- tx = usbatm_pop_transceiver(&instance->tx_channel);
- if (!tx)
+ if (!urb) {
+ urb = usbatm_pop_urb(&instance->tx_channel);
+ if (!urb)
break; /* no more senders */
- buffer = tx->urb->transfer_buffer;
- num_written = 0;
+ buffer = urb->transfer_buffer;
+ num_written = (urb->status == -EAGAIN) ?
+ urb->transfer_buffer_length : 0;
}
num_written += usbatm_write_cells(instance, skb,
buffer + num_written,
buf_size - num_written);
- vdbg("%s: wrote %u bytes from skb 0x%p to sender 0x%p",
- __func__, num_written, skb, tx);
+ vdbg("%s: wrote %u bytes from skb 0x%p to urb 0x%p",
+ __func__, num_written, skb, urb);
if (!UDSL_SKB(skb)->len) {
struct atm_vcc *vcc = UDSL_SKB(skb)->atm.vcc;
@@ -552,13 +549,12 @@ static void usbatm_tx_process(unsigned l
}
if (num_written == buf_size || (!skb && num_written)) {
- tx->urb->transfer_buffer_length = num_written;
+ urb->transfer_buffer_length = num_written;
- if (usbatm_submit_transceiver(tx))
+ if (usbatm_submit_urb(urb))
break;
- tx = NULL;
+ urb = NULL;
}
-
}
instance->current_skb = skb;
@@ -879,7 +875,7 @@ static int usbatm_atm_init(struct usbatm
/* submit all rx URBs */
for (i = 0; i < num_rcv_urbs; i++)
- usbatm_submit_transceiver(instance->transceivers + i);
+ usbatm_submit_urb(instance->urbs[i]);
return 0;
@@ -975,7 +971,9 @@ int usbatm_usb_probe(struct usb_interfac
intf->altsetting->desc.bInterfaceNumber);
/* instance init */
- if (!(instance = kmalloc(sizeof(*instance), GFP_KERNEL))) {
+ instance = kmalloc(sizeof(*instance) + sizeof(struct urb *) * (num_rcv_urbs + num_snd_urbs),
+ GFP_KERNEL);
+ if (!instance) {
dev_dbg(dev, "%s: no memory for instance data!\n", __func__);
return -ENOMEM;
}
@@ -1043,63 +1041,55 @@ int usbatm_usb_probe(struct usb_interfac
skb_queue_head_init(&instance->sndqueue);
- instance->transceivers = kmalloc(sizeof(*instance->transceivers) * (num_rcv_urbs + num_snd_urbs),
- GFP_KERNEL);
- if (!instance->transceivers) {
- dev_dbg(dev, "%s: no memory for transceivers!\n", __func__);
- goto fail_unbind;
- }
-
- memset(instance->transceivers, 0, sizeof(*instance->transceivers) * (num_rcv_urbs + num_snd_urbs));
-
for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
- struct usbatm_transceiver *trx = instance->transceivers + i;
+ struct urb *urb;
u8 *buffer;
unsigned int iso_packets = 0, iso_size = 0;
+ struct usbatm_channel *channel = i < num_rcv_urbs ?
+ &instance->rx_channel : &instance->tx_channel;
- trx->channel = i < num_rcv_urbs ? &instance->rx_channel : &instance->tx_channel;
-
- buffer = kmalloc(trx->channel->buf_size, GFP_KERNEL);
+ buffer = kmalloc(channel->buf_size, GFP_KERNEL);
if (!buffer) {
dev_dbg(dev, "%s: no memory for buffer %d!\n", __func__, i);
goto fail_unbind;
}
- memset(buffer, 0, trx->channel->buf_size);
+ memset(buffer, 0, channel->buf_size);
- if (usb_pipeisoc(trx->channel->endpoint)) {
+ if (usb_pipeisoc(channel->endpoint)) {
/* don't expect iso out endpoints */
- iso_size = usb_maxpacket(instance->usb_dev, trx->channel->endpoint, 0);
- iso_size -= iso_size % trx->channel->stride; /* alignment */
+ iso_size = usb_maxpacket(instance->usb_dev, channel->endpoint, 0);
+ iso_size -= iso_size % channel->stride; /* alignment */
BUG_ON(!iso_size);
- iso_packets = (trx->channel->buf_size - 1) / iso_size + 1;
+ iso_packets = (channel->buf_size - 1) / iso_size + 1;
}
- trx->urb = usb_alloc_urb(iso_packets, GFP_KERNEL);
- if (!trx->urb) {
+ urb = usb_alloc_urb(iso_packets, GFP_KERNEL);
+ if (!urb) {
dev_dbg(dev, "%s: no memory for urb %d!\n", __func__, i);
goto fail_unbind;
}
- usb_fill_bulk_urb(trx->urb, instance->usb_dev, trx->channel->endpoint,
- buffer, trx->channel->buf_size, usbatm_complete, trx);
+ usb_fill_bulk_urb(urb, instance->usb_dev, channel->endpoint,
+ buffer, channel->buf_size, usbatm_complete, channel);
if (iso_packets) {
int j;
- trx->urb->interval = 1;
- trx->urb->transfer_flags = URB_ISO_ASAP;
- trx->urb->number_of_packets = iso_packets;
+ urb->interval = 1;
+ urb->transfer_flags = URB_ISO_ASAP;
+ urb->number_of_packets = iso_packets;
for (j = 0; j < iso_packets; j++) {
- trx->urb->iso_frame_desc[j].offset = iso_size * j;
- trx->urb->iso_frame_desc[j].length = min_t(int, iso_size,
- trx->channel->buf_size - trx->urb->iso_frame_desc[j].offset);
+ urb->iso_frame_desc[j].offset = iso_size * j;
+ urb->iso_frame_desc[j].length = min_t(int, iso_size,
+ channel->buf_size - urb->iso_frame_desc[j].offset);
}
}
/* put all tx URBs on the list of spares */
if (i >= num_rcv_urbs)
- list_add_tail(&trx->list, &trx->channel->list);
+ list_add_tail(&urb->urb_list, &channel->list);
- vdbg("%s: alloced trx 0x%p buffer 0x%p buf size %u urb 0x%p",
- __func__, trx, trx->urb->transfer_buffer, trx->urb->transfer_buffer_length, trx->urb);
+ vdbg("%s: alloced buffer 0x%p buf size %u urb 0x%p",
+ __func__, urb->transfer_buffer, urb->transfer_buffer_length, urb);
+ instance->urbs[i] = urb;
}
if (need_heavy && driver->heavy_init) {
@@ -1121,13 +1111,11 @@ int usbatm_usb_probe(struct usb_interfac
if (instance->driver->unbind)
instance->driver->unbind(instance, intf);
fail_free:
- if (instance->transceivers)
- for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
- kfree(instance->transceivers[i].urb->transfer_buffer);
- usb_free_urb(instance->transceivers[i].urb);
- }
+ for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
+ kfree(instance->urbs[i]->transfer_buffer);
+ usb_free_urb(instance->urbs[i]);
+ }
- kfree(instance->transceivers);
kfree (instance);
return error;
@@ -1160,7 +1148,7 @@ void usbatm_usb_disconnect(struct usb_in
tasklet_disable(&instance->tx_channel.tasklet);
for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++)
- usb_kill_urb(instance->transceivers[i].urb);
+ usb_kill_urb(instance->urbs[i]);
del_timer_sync(&instance->rx_channel.delay);
del_timer_sync(&instance->tx_channel.delay);
@@ -1184,12 +1172,10 @@ void usbatm_usb_disconnect(struct usb_in
tasklet_enable(&instance->tx_channel.tasklet);
for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
- kfree(instance->transceivers[i].urb->transfer_buffer);
- usb_free_urb(instance->transceivers[i].urb);
+ kfree(instance->urbs[i]->transfer_buffer);
+ usb_free_urb(instance->urbs[i]);
}
- kfree(instance->transceivers);
-
/* ATM finalize */
if (instance->atm_dev)
shutdown_atm_dev(instance->atm_dev);
More information about the Usbatm
mailing list