[PATCH 22/32] rt2x00: Allocate ring structures in single array
From: Ivo van Doorn <[EMAIL PROTECTED]> Allocate all ring structures seperately and as an array of rings. This will make the rt2x00_pci and rt2x00_usb structures more generic for all rt2x00 modules. Use seperate function to convert a dscape ring ID to the address of the actual ring. Signed-off-by: Ivo van Doorn <[EMAIL PROTECTED]> Available on server: http://mendiosus.nl/rt2x00/rt2x00-22-rings.diff pgp37E3iQh6EF.pgp Description: PGP signature
Re: [PATCH 22/32] rt2x00: Allocate ring structures in single array
On Fri, 28 Apr 2006 00:03:13 +0200, Ivo van Doorn wrote:
> Use seperate function to convert a dscape ring ID
> to the address of the actual ring.
Just minor things:
> [...]
> @@ -1386,20 +1412,19 @@ rt2400pci_tx(struct net_device *net_dev,
>
> rt2x00_register_read(rt2x00pci, TXCSR0, ®);
>
> - if (control->queue == IEEE80211_TX_QUEUE_DATA0) {
> - ring = &rt2x00pci->prio;
> - rt2x00_set_field32(®, TXCSR0_KICK_PRIO, 1);
> - } else if (control->queue == IEEE80211_TX_QUEUE_DATA1) {
> - ring = &rt2x00pci->tx;
> - rt2x00_set_field32(®, TXCSR0_KICK_TX, 1);
> - } else if (control->queue == IEEE80211_TX_QUEUE_AFTER_BEACON) {
> - ring = &rt2x00pci->atim;
> - rt2x00_set_field32(®, TXCSR0_KICK_ATIM, 1);
> - } else {
> - ERROR("Frame received for invalid queue.");
> + /*
> + * Determine which ring to put packet on.
> + */
> + ring = rt2x00pci_get_ring(rt2x00pci, control->queue);
> + if (unlikely(!ring)) {
Should not happen. Maybe some message that user should report a bug?
> + ERROR("Attempt to send packet over invalid queue %d.\n",
> + control->queue);
> return NET_RX_DROP;
> }
>
> + if (rt2x00_ring_full(ring))
> + return NET_RX_DROP;
NET_XMIT_DROP?
> +
> entry = rt2x00_get_data_entry(ring);
> txd = entry->desc_addr;
>
> [...]
> @@ -1780,14 +1813,17 @@ rt2400pci_conf_tx(struct net_device *net
> int queue, const struct ieee80211_tx_queue_params *params)
> {
> struct rt2x00_pci *rt2x00pci = ieee80211_dev_hw_data(net_dev);
> - struct data_ring*ring;
> + struct data_ring*ring = &rt2x00pci->ring[RING_TX];
>
> - if (queue == IEEE80211_TX_QUEUE_DATA0)
> - ring = &rt2x00pci->prio;
> - else if (queue == IEEE80211_TX_QUEUE_DATA1)
> - ring = &rt2x00pci->tx;
> - else
> - return -EINVAL;
> + /*
> + * We don't support variating cw_min and cw_max variables
> + * per queue. So by default we only configure the TX queue,
> + * and ignore all other configurations.
> + */
> + if (queue != IEEE80211_TX_QUEUE_DATA0) {
> + NOTICE("Ignoring configuration for queue %d.\n", queue);
> + return 0;
Is there a reason for not returning a proper error code?
> + }
>
> memcpy(&ring->tx_params, params, sizeof(*params));
>
Ditto for rt2500pci and rt2500usb.
Thanks,
--
Jiri Benc
SUSE Labs
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 22/32] rt2x00: Allocate ring structures in single array
On Friday 28 April 2006 12:52, Jiri Benc wrote:
> On Fri, 28 Apr 2006 00:03:13 +0200, Ivo van Doorn wrote:
> > Use seperate function to convert a dscape ring ID
> > to the address of the actual ring.
>
> Just minor things:
>
> > [...]
> > @@ -1386,20 +1412,19 @@ rt2400pci_tx(struct net_device *net_dev,
> >
> > rt2x00_register_read(rt2x00pci, TXCSR0, ®);
> >
> > - if (control->queue == IEEE80211_TX_QUEUE_DATA0) {
> > - ring = &rt2x00pci->prio;
> > - rt2x00_set_field32(®, TXCSR0_KICK_PRIO, 1);
> > - } else if (control->queue == IEEE80211_TX_QUEUE_DATA1) {
> > - ring = &rt2x00pci->tx;
> > - rt2x00_set_field32(®, TXCSR0_KICK_TX, 1);
> > - } else if (control->queue == IEEE80211_TX_QUEUE_AFTER_BEACON) {
> > - ring = &rt2x00pci->atim;
> > - rt2x00_set_field32(®, TXCSR0_KICK_ATIM, 1);
> > - } else {
> > - ERROR("Frame received for invalid queue.");
> > + /*
> > +* Determine which ring to put packet on.
> > +*/
> > + ring = rt2x00pci_get_ring(rt2x00pci, control->queue);
> > + if (unlikely(!ring)) {
>
> Should not happen. Maybe some message that user should report a bug?
I am not really fan on such messages. But when the control->queue is indeed
invalid, it is a serious bug. Perhaps a BUG() should be used as well?
If so, I'll put it into a new patch along with the fixes below.
> > + ERROR("Attempt to send packet over invalid queue %d.\n",
> > + control->queue);
> > return NET_RX_DROP;
> > }
> >
> > + if (rt2x00_ring_full(ring))
> > + return NET_RX_DROP;
>
> NET_XMIT_DROP?
Absolutely true, I'll fix this as well.
> > +
> > entry = rt2x00_get_data_entry(ring);
> > txd = entry->desc_addr;
> >
> > [...]
> > @@ -1780,14 +1813,17 @@ rt2400pci_conf_tx(struct net_device *net
> > int queue, const struct ieee80211_tx_queue_params *params)
> > {
> > struct rt2x00_pci *rt2x00pci = ieee80211_dev_hw_data(net_dev);
> > - struct data_ring*ring;
> > + struct data_ring*ring = &rt2x00pci->ring[RING_TX];
> >
> > - if (queue == IEEE80211_TX_QUEUE_DATA0)
> > - ring = &rt2x00pci->prio;
> > - else if (queue == IEEE80211_TX_QUEUE_DATA1)
> > - ring = &rt2x00pci->tx;
> > - else
> > - return -EINVAL;
> > + /*
> > +* We don't support variating cw_min and cw_max variables
> > +* per queue. So by default we only configure the TX queue,
> > +* and ignore all other configurations.
> > +*/
> > + if (queue != IEEE80211_TX_QUEUE_DATA0) {
> > + NOTICE("Ignoring configuration for queue %d.\n", queue);
> > + return 0;
>
> Is there a reason for not returning a proper error code?
Good question, in rt2400pci the behaviour is something that is not
somethign the dscape could expect. Since we don't allow the
configuration of all available tx queues.
I had made it return 0, since I wasn't sure how the stack would
appreciate this behaviour.
But I have checked this in the stack since then, and apparently,
tx_conf is called without any notice of available tx queues.
Only when WMM is available the return value is checked,
so it is safe to return a error value.
But this behaviour should never have appeared in rt2500pci and rt2500usb,
so I'll put this fix in the patch as well.
Thanks.
pgpX3vN5g2DzR.pgp
Description: PGP signature
Re: [PATCH 22/32] rt2x00: Allocate ring structures in single array
On Fri, 28 Apr 2006 16:25:09 +0200, Ivo van Doorn wrote: > I am not really fan on such messages. But when the control->queue is indeed > invalid, it is a serious bug. Perhaps a BUG() should be used as well? > If so, I'll put it into a new patch along with the fixes below. BUG() is not a good idea, this is not critical enough to panic the kernel. -- Jiri Benc SUSE Labs - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 22/32] rt2x00: Allocate ring structures in single array
On Friday 28 April 2006 16:30, Jiri Benc wrote: > On Fri, 28 Apr 2006 16:25:09 +0200, Ivo van Doorn wrote: > > I am not really fan on such messages. But when the control->queue is indeed > > invalid, it is a serious bug. Perhaps a BUG() should be used as well? > > If so, I'll put it into a new patch along with the fixes below. > > BUG() is not a good idea, this is not critical enough to panic the kernel. Ok, I"ll just change the debug message then. :) Thanks. pgpHaIquBhJ92.pgp Description: PGP signature
