Re: [PATCH 22/32] rt2x00: Allocate ring structures in single array

[Jiri Benc]

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, &reg);
>  
> -     if (control->queue == IEEE80211_TX_QUEUE_DATA0) {
> -             ring = &rt2x00pci->prio;
> -             rt2x00_set_field32(&reg, TXCSR0_KICK_PRIO, 1);
> -     } else if (control->queue == IEEE80211_TX_QUEUE_DATA1) {
> -             ring = &rt2x00pci->tx;
> -             rt2x00_set_field32(&reg, TXCSR0_KICK_TX, 1);
> -     } else if (control->queue == IEEE80211_TX_QUEUE_AFTER_BEACON) {
> -             ring = &rt2x00pci->atim;
> -             rt2x00_set_field32(&reg, 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