All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Seth Forshee <seth.forshee@canonical.com>
Cc: linux-wireless@vger.kernel.org,
	"John W. Linville" <linville@tuxdriver.com>,
	Stanislaw Gruszka <sgruszka@redhat.com>,
	"Luis R. Rodriguez" <mcgrof@qca.qualcomm.com>,
	Jouni Malinen <jouni@qca.qualcomm.com>,
	Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>,
	Senthil Balasubramanian <senthilb@qca.qualcomm.com>,
	Christian Lamparter <chunkeey@googlemail.com>,
	Ivo van Doorn <IvDoorn@gmail.com>,
	Gertjan van Wingerde <gwingerde@gmail.com>,
	Helmut Schaa <helmut.schaa@googlemail.com>,
	Larry Finger <Larry.Finger@lwfinger.net>,
	Chaoming Li <chaoming_li@realsil.com.cn>,
	Arend van Spriel <arend@broadcom.com>,
	Luciano Coelho <coelho@ti.com>,
	ath9k-devel@venema.h4ckr.net, brcm80211-dev-list@broadcom.com,
	users@rt2x00.serialmonkey.com
Subject: Re: [PATCH 5/7] mac80211: Expand powersave configuration flag to be two bits
Date: Thu, 31 Jan 2013 18:50:34 +0100	[thread overview]
Message-ID: <1359654634.8415.101.camel@jlt4.sipsolutions.net> (raw)
In-Reply-To: <20130131171826.GG28799@thinkpad-t410>

On Thu, 2013-01-31 at 11:18 -0600, Seth Forshee wrote:

> > > Actually one of the last bugs I fixed before sending these was a place
> > > where I had used disabled instead of !enabled, and the frames ended up
> > > with PM set when it shouldn't have been.
> > > 
> > > I agree though that the distinction is confusing. Maybe some better
> > > state names are needed. Perhaps awake, offchannel, and doze?
> > 
> > I think what you really want is to distinguish between "HW can go to
> > powersave" and "PM bit should be set"? That's pretty much what your
> > CONF_PS_ENABLED and CONF_PS_OFFCHANNEL means, respectively, but maybe
> 
> Correct, with the understanding that "HW can go to powersave" also
> implies "PM bit should be set."
> 
> Another approach would be to keep the CONF_PS flag the same and add a
> CONF_PM flag or similar. I didn't go with this approach because CONF_PS
> && !CONF_PM really doesn't make any sense, which really doesn't help
> with reducing confusion. The advantage is that it separates setting PM
> from PS for those driver that don't support PS but need to configure the
> hardware to set PM for off-channel.

Good point, that'd work too. PS && !PM would never be used, I guess?
It'd also have the advantage of not having to touch all the drivers?

It doesn't really matter all that much to me though, I just think what
you have right now is (too) confusing.

johannes


WARNING: multiple messages have this Message-ID (diff)
From: Johannes Berg <johannes@sipsolutions.net>
To: ath9k-devel@lists.ath9k.org
Subject: [ath9k-devel] [PATCH 5/7] mac80211: Expand powersave configuration flag to be two bits
Date: Thu, 31 Jan 2013 18:50:34 +0100	[thread overview]
Message-ID: <1359654634.8415.101.camel@jlt4.sipsolutions.net> (raw)
In-Reply-To: <20130131171826.GG28799@thinkpad-t410>

On Thu, 2013-01-31 at 11:18 -0600, Seth Forshee wrote:

> > > Actually one of the last bugs I fixed before sending these was a place
> > > where I had used disabled instead of !enabled, and the frames ended up
> > > with PM set when it shouldn't have been.
> > > 
> > > I agree though that the distinction is confusing. Maybe some better
> > > state names are needed. Perhaps awake, offchannel, and doze?
> > 
> > I think what you really want is to distinguish between "HW can go to
> > powersave" and "PM bit should be set"? That's pretty much what your
> > CONF_PS_ENABLED and CONF_PS_OFFCHANNEL means, respectively, but maybe
> 
> Correct, with the understanding that "HW can go to powersave" also
> implies "PM bit should be set."
> 
> Another approach would be to keep the CONF_PS flag the same and add a
> CONF_PM flag or similar. I didn't go with this approach because CONF_PS
> && !CONF_PM really doesn't make any sense, which really doesn't help
> with reducing confusion. The advantage is that it separates setting PM
> from PS for those driver that don't support PS but need to configure the
> hardware to set PM for off-channel.

Good point, that'd work too. PS && !PM would never be used, I guess?
It'd also have the advantage of not having to touch all the drivers?

It doesn't really matter all that much to me though, I just think what
you have right now is (too) confusing.

johannes

  reply	other threads:[~2013-01-31 17:50 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-29 23:47 [PATCH 0/7] Improvements to software scanning Seth Forshee
2013-01-29 23:47 ` [PATCH 1/7] mac80211: Return a status for tx operations Seth Forshee
2013-01-29 23:47 ` [PATCH 2/7] mac80211: Fix tx queue handling during scans Seth Forshee
2013-01-31 15:14   ` Johannes Berg
2013-01-31 16:14     ` Seth Forshee
2013-01-29 23:47 ` [PATCH 3/7] mac80211: Improve error handling for off-channel operation Seth Forshee
2013-01-31 15:15   ` Johannes Berg
2013-01-31 16:17     ` Seth Forshee
2013-01-29 23:47 ` [PATCH 4/7] mac80211: Add flushes before going off-channel Seth Forshee
2013-01-29 23:47 ` [PATCH 5/7] mac80211: Expand powersave configuration flag to be two bits Seth Forshee
2013-01-29 23:47   ` [ath9k-devel] " Seth Forshee
2013-01-31 15:20   ` Johannes Berg
2013-01-31 15:20     ` [ath9k-devel] " Johannes Berg
2013-01-31 16:33     ` Seth Forshee
2013-01-31 16:33       ` [ath9k-devel] " Seth Forshee
2013-01-31 16:53       ` Johannes Berg
2013-01-31 16:53         ` [ath9k-devel] " Johannes Berg
2013-01-31 17:18         ` Seth Forshee
2013-01-31 17:18           ` [ath9k-devel] " Seth Forshee
2013-01-31 17:50           ` Johannes Berg [this message]
2013-01-31 17:50             ` Johannes Berg
2013-02-05 22:51             ` Seth Forshee
2013-02-05 22:51               ` [ath9k-devel] " Seth Forshee
2013-02-06 16:48               ` Johannes Berg
2013-02-06 16:48                 ` [ath9k-devel] " Johannes Berg
2013-02-06 17:09                 ` Seth Forshee
2013-02-06 17:09                   ` [ath9k-devel] " Seth Forshee
2013-02-06 17:44                   ` Johannes Berg
2013-02-06 17:44                     ` [ath9k-devel] " Johannes Berg
2013-02-06 18:02                     ` Seth Forshee
2013-02-06 18:02                       ` [ath9k-devel] " Seth Forshee
2013-02-06 21:30                       ` Johannes Berg
2013-02-06 21:30                         ` [ath9k-devel] " Johannes Berg
2013-01-29 23:47 ` [PATCH 6/7] mac80211: Add off-channel powersave state Seth Forshee
2013-01-29 23:47 ` [PATCH 7/7] brcmsmac: Add support for off-channel powersave Seth Forshee
2013-01-29 23:56   ` Julian Calaby
2013-01-30  5:28     ` Seth Forshee
2013-01-30 19:34 ` [PATCH 0/7] Improvements to software scanning John W. Linville
2013-01-30 21:27 ` Arend van Spriel
2013-01-30 21:53   ` Seth Forshee
2013-01-31 15:04 ` Johannes Berg
2013-01-31 15:08   ` Johannes Berg
2013-01-31 16:02     ` Seth Forshee
2013-01-31 15:48   ` Seth Forshee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1359654634.8415.101.camel@jlt4.sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=IvDoorn@gmail.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=arend@broadcom.com \
    --cc=ath9k-devel@venema.h4ckr.net \
    --cc=brcm80211-dev-list@broadcom.com \
    --cc=chaoming_li@realsil.com.cn \
    --cc=chunkeey@googlemail.com \
    --cc=coelho@ti.com \
    --cc=gwingerde@gmail.com \
    --cc=helmut.schaa@googlemail.com \
    --cc=jouni@qca.qualcomm.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=mcgrof@qca.qualcomm.com \
    --cc=senthilb@qca.qualcomm.com \
    --cc=seth.forshee@canonical.com \
    --cc=sgruszka@redhat.com \
    --cc=users@rt2x00.serialmonkey.com \
    --cc=vthiagar@qca.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.