On Fri 26.10.07 10:37, Luis R. Rodriguez wrote:
> On 10/26/07, Ulrich Meis <[EMAIL PROTECTED]> wrote:
> > This patch fixes the setup of transmit descriptors. tx_control_1 is set
> > in ath5k_hw_setup_{2,4}word_tx_desc but was subsequently overriden by
> > ath5k_hw_fill_{2,4}word_tx_desc. The victims were FRAME_TYPE and NO_ACK.
> > The missing no_ack in broadcast frames caused them to be retried up to
> > the retry_limit(1+4=5 transmissions by default). The changes to the 2
> > words variant are identical to 4 words but untested.
> >
> > Signed-off-by: Ulrich Meis <[EMAIL PROTECTED]>
> > ---
> > diff --git a/drivers/net/wireless/ath5k/hw.c
> > b/drivers/net/wireless/ath5k/hw.c
> > index c8d1fbd..f9c0165 100644
> > --- a/drivers/net/wireless/ath5k/hw.c
> > +++ b/drivers/net/wireless/ath5k/hw.c
> > @@ -3702,10 +3702,11 @@ static int ath5k_hw_fill_2word_tx_desc(struct
> > ath5k_hw *ah,
> > /* Clear status descriptor */
> > memset(desc->ds_hw, 0, sizeof(desc->ds_hw));
> >
> > - /* Validate segment length and initialize the descriptor */
> > - tx_desc->tx_control_1 = segment_length &
> > AR5K_2W_TX_DESC_CTL1_BUF_LEN;
> > - if (tx_desc->tx_control_1 != segment_length)
> > + /* Validate segment length and update the descriptor */
> > + if (segment_length & ~AR5K_2W_TX_DESC_CTL1_BUF_LEN)
> > return -EINVAL;
> > + tx_desc->tx_control_1 = (tx_desc->tx_control_1 &
> > + ~AR5K_2W_TX_DESC_CTL1_BUF_LEN) | segment_length;
> >
> > if (first_segment != true)
> > tx_desc->tx_control_0 &= ~AR5K_2W_TX_DESC_CTL0_FRAME_LEN;
> > @@ -3734,10 +3735,11 @@ static int ath5k_hw_fill_4word_tx_desc(struct
> > ath5k_hw *ah,
> > /* Clear status descriptor */
> > memset(tx_status, 0, sizeof(struct ath5k_hw_tx_status));
> >
> > - /* Validate segment length and initialize the descriptor */
> > - tx_desc->tx_control_1 = segment_length &
> > AR5K_4W_TX_DESC_CTL1_BUF_LEN;
> > - if (tx_desc->tx_control_1 != segment_length)
> > + /* Validate segment length and update the descriptor */
> > + if (segment_length & ~AR5K_4W_TX_DESC_CTL1_BUF_LEN)
> > return -EINVAL;
> > + tx_desc->tx_control_1 = (tx_desc->tx_control_1 &
> > + ~AR5K_4W_TX_DESC_CTL1_BUF_LEN) | segment_length;
> >
> > if (first_segment != true)
> > tx_desc->tx_control_0 &= ~AR5K_4W_TX_DESC_CTL0_FRAME_LEN;
>
> Excellent catch! How about we get rid of the ah_fill_tx_desc's. Don't
> see a reason to keep them separate from the setup_tx_desc.
Thanks, it took a while to figure that out :)
I'm new to the code, so I'm not sure about the usefulness of those
functions. However, it seems that they exist for the superg "fast
frames" feature. I had a look into the madwifi code and saw only one
call to the fill function that wasn't preceded by setup and that was in
FF code in if_ath.c:ath_tx_start(). It seems that frames eligible for FF
transmission are queued seperately and once a partner is found, they are
transmitted together. The second frame becomes the first and its
descriptor is setup and filled. The other frame also gets a descriptor
but it is only filled, not setup. I guess that means that it contains
some random values that the hardware ignores.
By the way, is anyone working on the implementation of "fast frames"?
Do you just have to set AR5K_4W_TX_DESC_CTL1_MORE and the device will
fetch the next descriptor and transmit the two frames together? If not,
what else is needed?
Thanks,
Uli
_______________________________________________
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel
