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.

  Luis
_______________________________________________
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel

Reply via email to