Re: [PATCH 6/7] p54: refractor beacon_update and stop path

["John W. Linville" <linville@tuxdriver.com>]

On Fri, Mar 06, 2009 at 04:38:16PM +0100, Michael Buesch wrote:
> On Thursday 05 March 2009 21:33:32 Christian Lamparter wrote:
> > According to the LMAC API Section 3.2.2, we don't have to cancel the previous
> > beacon manually. The firmware will automatically return it to the driver
> > through the feedback mechanism. 
> > And while we're at it, we can remove some left over code as well.
> > 
> > Signed-off-by: Christian Lamparter <chunkeey@web.de>
> > ---
> > diff -Nurp a/drivers/net/wireless/p54/p54common.c b/drivers/net/wireless/p54/p54common.c
> > --- a/drivers/net/wireless/p54/p54common.c	2009-03-05 15:42:57.000000000 +0100
> > +++ b/drivers/net/wireless/p54/p54common.c	2009-03-05 15:50:10.000000000 +0100
> > @@ -2015,18 +2015,14 @@ static int p54_beacon_update(struct ieee
> >  	struct sk_buff *beacon;
> >  	int ret;
> >  
> > -	if (priv->cached_beacon) {
> > -		p54_tx_cancel(dev, priv->cached_beacon);
> > -		/* wait for the last beacon the be freed */
> > -		msleep(10);
> > -	}
> > -
> >  	beacon = ieee80211_beacon_get(dev, vif);
> >  	if (!beacon)
> >  		return -ENOMEM;
> >  	ret = p54_beacon_tim(beacon);
> >  	if (ret)
> >  		return ret;
> > +
> > +	/* the firmware will automatically cancel old beacons. */ 
> >  	ret = p54_tx(dev, beacon);
> >  	if (ret)
> >  		return ret;
> > @@ -2072,17 +2068,21 @@ out:
> >  static void p54_stop(struct ieee80211_hw *dev)
> >  {
> >  	struct p54_common *priv = dev->priv;
> > -	struct sk_buff *skb;
> >  
> >  	mutex_lock(&priv->conf_mutex);
> >  	priv->mode = NL80211_IFTYPE_UNSPECIFIED;
> > +	p54_setup_mac(dev);
> >  	cancel_delayed_work_sync(&priv->work);
> > -	if (priv->cached_beacon)
> > -		p54_tx_cancel(dev, priv->cached_beacon);
> > +
> > +	/*
> > +	 * p54spi uses mac80211's workqueue to write the frames into
> > +	 * device's memory. Therefore, we should give the queue a chance
> > +	 * to complete all outstanding work, before the device is stopped.
> > +	 */
> > +	msleep(25);
> 
> What about flush_workqueue()?

I agree, we should use an actual API here and not just "wait long
enough (we hope)"...

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.