msm8916 + wcn3620 mainline support?

Andy Green andy.green at linaro.org
Thu Jan 22 15:42:41 PST 2015


On 21 January 2015 at 01:14, Bob Copeland <me at bobcopeland.com> wrote:
> On Tue, Jan 20, 2015 at 08:05:04AM +0800, Andy Green wrote:
>> > RE stability problems, I have a couple of locking-related patches that
>> > aren't upstream yet (or in wcn36xx-devel) that might be worth a look.
>> > I'll clean them up and send along for discussion.
>>
>> I'd be very happy to give them a try.
>
> Give this a go - it looked to me like it was possible for ring list
> to get corrupted when driver and irq were both manipulating it, and
> existing skb lock was by itself insufficient.

This seems to help it not drop dead... three overnight tests in a row
the link was still associated and able to ping the following morning.

It doesn't change the behaviour about scan itself blocking the link
for 10s, but it seems it can survive a lot longer at least.

And no new problems are coming, so it sounds good to me...  FWIW

Tested-by: Andy Green <andy.green at linaro.org>

-Andy

> From 96ff443d35aef0f76bbdec03028fee062b6fd84d Mon Sep 17 00:00:00 2001
> From: Bob Copeland <me at bobcopeland.com>
> Date: Tue, 20 Jan 2015 12:11:59 -0500
> Subject: [PATCH] wcn36xx: add locking around ring buffer accesses
>
> Signed-off-by: Bob Copeland <me at bobcopeland.com>
> ---
>  drivers/net/wireless/ath/wcn36xx/dxe.c |   27 +++++++++++++++++++--------
>  drivers/net/wireless/ath/wcn36xx/dxe.h |    1 +
>  2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c
> index 086549b..26085f7 100644
> --- a/drivers/net/wireless/ath/wcn36xx/dxe.c
> +++ b/drivers/net/wireless/ath/wcn36xx/dxe.c
> @@ -79,6 +79,7 @@ static int wcn36xx_dxe_allocate_ctl_block(struct wcn36xx_dxe_ch *ch)
>         struct wcn36xx_dxe_ctl *cur_ctl = NULL;
>         int i;
>
> +       spin_lock_init(&ch->lock);
>         for (i = 0; i < ch->desc_num; i++) {
>                 cur_ctl = kzalloc(sizeof(*cur_ctl), GFP_KERNEL);
>                 if (!cur_ctl)
> @@ -345,7 +346,7 @@ void wcn36xx_dxe_tx_ack_ind(struct wcn36xx *wcn, u32 status)
>
>  static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch)
>  {
> -       struct wcn36xx_dxe_ctl *ctl = ch->tail_blk_ctl;
> +       struct wcn36xx_dxe_ctl *ctl;
>         struct ieee80211_tx_info *info;
>         unsigned long flags;
>
> @@ -354,6 +355,8 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch)
>          * completely full head and tail are pointing to the same element
>          * and while-do will not make any cycles.
>          */
> +       spin_lock_irqsave(&ch->lock, flags);
> +       ctl = ch->tail_blk_ctl;
>         do {
>                 if (ctl->desc->ctrl & WCN36XX_DXE_CTRL_VALID_MASK)
>                         break;
> @@ -365,12 +368,12 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch)
>                                 /* Keep frame until TX status comes */
>                                 ieee80211_free_txskb(wcn->hw, ctl->skb);
>                         }
> -                       spin_lock_irqsave(&ctl->skb_lock, flags);
> +                       spin_lock(&ctl->skb_lock);
>                         if (wcn->queues_stopped) {
>                                 wcn->queues_stopped = false;
>                                 ieee80211_wake_queues(wcn->hw);
>                         }
> -                       spin_unlock_irqrestore(&ctl->skb_lock, flags);
> +                       spin_unlock(&ctl->skb_lock);
>
>                         ctl->skb = NULL;
>                 }
> @@ -379,6 +382,7 @@ static void reap_tx_dxes(struct wcn36xx *wcn, struct wcn36xx_dxe_ch *ch)
>                !(ctl->desc->ctrl & WCN36XX_DXE_CTRL_VALID_MASK));
>
>         ch->tail_blk_ctl = ctl;
> +       spin_unlock_irqrestore(&ch->lock, flags);
>  }
>
>  static irqreturn_t wcn36xx_irq_tx_complete(int irq, void *dev)
> @@ -596,12 +600,14 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
>         struct wcn36xx_dxe_desc *desc = NULL;
>         struct wcn36xx_dxe_ch *ch = NULL;
>         unsigned long flags;
> +       int ret;
>
>         ch = is_low ? &wcn->dxe_tx_l_ch : &wcn->dxe_tx_h_ch;
>
> +       spin_lock_irqsave(&ch->lock, flags);
>         ctl = ch->head_blk_ctl;
>
> -       spin_lock_irqsave(&ctl->next->skb_lock, flags);
> +       spin_lock(&ctl->next->skb_lock);
>
>         /*
>          * If skb is not null that means that we reached the tail of the ring
> @@ -611,10 +617,11 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
>         if (NULL != ctl->next->skb) {
>                 ieee80211_stop_queues(wcn->hw);
>                 wcn->queues_stopped = true;
> -               spin_unlock_irqrestore(&ctl->next->skb_lock, flags);
> +               spin_unlock(&ctl->next->skb_lock);
> +               spin_unlock_irqrestore(&ch->lock, flags);
>                 return -EBUSY;
>         }
> -       spin_unlock_irqrestore(&ctl->next->skb_lock, flags);
> +       spin_unlock(&ctl->next->skb_lock);
>
>         ctl->skb = NULL;
>         desc = ctl->desc;
> @@ -640,7 +647,8 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
>         desc = ctl->desc;
>         if (ctl->bd_cpu_addr) {
>                 wcn36xx_err("bd_cpu_addr cannot be NULL for skb DXE\n");
> -               return -EINVAL;
> +               ret = -EINVAL;
> +               goto unlock;
>         }
>
>         desc->src_addr_l = dma_map_single(NULL,
> @@ -679,7 +687,10 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
>                         ch->reg_ctrl, ch->def_ctrl);
>         }
>
> -       return 0;
> +       ret = 0;
> +unlock:
> +       spin_unlock_irqrestore(&ch->lock, flags);
> +       return ret;
>  }
>
>  int wcn36xx_dxe_init(struct wcn36xx *wcn)
> diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.h b/drivers/net/wireless/ath/wcn36xx/dxe.h
> index 35ee7e9..f869fd3 100644
> --- a/drivers/net/wireless/ath/wcn36xx/dxe.h
> +++ b/drivers/net/wireless/ath/wcn36xx/dxe.h
> @@ -243,6 +243,7 @@ struct wcn36xx_dxe_ctl {
>  };
>
>  struct wcn36xx_dxe_ch {
> +       spinlock_t                      lock;
>         enum wcn36xx_dxe_ch_type        ch_type;
>         void                            *cpu_addr;
>         dma_addr_t                      dma_addr;
> --
> 1.7.10.4
>
>
>
>
> --
> Bob Copeland %% http://bobcopeland.com/



More information about the wcn36xx mailing list