DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 51A102083A
Date:   Wed, 26 Sep 2018 12:04:28 +0300
From:   Ali MJ Al-Nasrawy <alimjalnasrawy@gmail.com>
To:     Arend van Spriel <arend.vanspriel@broadcom.com>
Cc:     linux-wireless@vger.kernel.org,
        brcm80211-dev-list.pdl@broadcom.com, brcm80211-dev-list@cypress.com
Subject: Re: [PATCH v3] brcmsmac: AP mode: update beacon when TIM changes
Message-ID: <20180926120428.00e75231@manjaro>
In-Reply-To: <20180923095425.10292-1-alimjalnasrawy@gmail.com>
References: <20180911172618.13049-1-alimjalnasrawy@gmail.com>
        <20180923095425.10292-1-alimjalnasrawy@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Hi,

I suggest here some improvements to this patch but I don't have strong
opinion about them so any feedback is appreciated:

-split it into two: one for storing the current vif into brcms_c_info
and also setting it to NULL on remove_interface to avoid dangling
pointers; and an other one for actually updating the beacon; I believe
them to be separate changes.

On Sun, 23 Sep 2018 12:54:25 +0300
Ali MJ Al-Nasrawy <alimjalnasrawy@gmail.com> wrote: 
> +static int brcms_ops_beacon_set_tim(struct ieee80211_hw *hw,
> +				 struct ieee80211_sta *sta, bool set)
> +{
> +	struct brcms_info *wl = hw->priv;
> +	struct sk_buff *beacon;
> +	u16 tim_offset = 0;
> +
> +	beacon = ieee80211_beacon_get_tim(hw, wl->wlc->vif,
> +					  &tim_offset, NULL);
> +	if (beacon){
> +		spin_lock_bh(&wl->lock);
> +		brcms_c_set_new_beacon(wl->wlc, beacon, tim_offset,
> +
> wl->wlc->vif->bss_conf.dtim_period);
> +		spin_unlock_bh(&wl->lock);
> +	}
> +
> +	return 0;
> +}
> +

-on brcms_ops_beacon_set_tim, include ieee80211_beacon_get_tim call in
the lock context and also check for vif to be null before passing it as
argument.