* [PATCH 6/7] p54: refractor beacon_update and stop path
@ 2009-03-05 20:33 Christian Lamparter
2009-03-06 15:38 ` Michael Buesch
0 siblings, 1 reply; 3+ messages in thread
From: Christian Lamparter @ 2009-03-05 20:33 UTC (permalink / raw)
To: linux-wireless; +Cc: John W. Linville
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);
priv->stop(dev);
- while ((skb = skb_dequeue(&priv->tx_queue)))
- kfree_skb(skb);
+ skb_queue_purge(&priv->tx_queue);
priv->cached_beacon = NULL;
priv->tsf_high32 = priv->tsf_low32 = 0;
mutex_unlock(&priv->conf_mutex);
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 6/7] p54: refractor beacon_update and stop path
2009-03-05 20:33 [PATCH 6/7] p54: refractor beacon_update and stop path Christian Lamparter
@ 2009-03-06 15:38 ` Michael Buesch
2009-03-17 19:59 ` John W. Linville
0 siblings, 1 reply; 3+ messages in thread
From: Michael Buesch @ 2009-03-06 15:38 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless, John W. Linville
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()?
But is it even possible for the work to execute here? Does the work take
the conf_mutex?
>
> priv->stop(dev);
> - while ((skb = skb_dequeue(&priv->tx_queue)))
> - kfree_skb(skb);
> + skb_queue_purge(&priv->tx_queue);
> priv->cached_beacon = NULL;
> priv->tsf_high32 = priv->tsf_low32 = 0;
> mutex_unlock(&priv->conf_mutex);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
Greetings, Michael.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 6/7] p54: refractor beacon_update and stop path
2009-03-06 15:38 ` Michael Buesch
@ 2009-03-17 19:59 ` John W. Linville
0 siblings, 0 replies; 3+ messages in thread
From: John W. Linville @ 2009-03-17 19:59 UTC (permalink / raw)
To: Michael Buesch; +Cc: Christian Lamparter, linux-wireless
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-03-17 20:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-05 20:33 [PATCH 6/7] p54: refractor beacon_update and stop path Christian Lamparter
2009-03-06 15:38 ` Michael Buesch
2009-03-17 19:59 ` John W. Linville
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).