2008-09-24 06:02:27

by Zhu Yi

[permalink] [raw]
Subject: [PATCH] iwl3945: added channel sysfs entry

From: Abhijeet Kolekar <[email protected]>

The patch adds channels sysfs entry for iwl3945. Make it consistent
with iwlagn.

Signed-off-by: Abhijeet Kolekar <[email protected]>
Signed-off-by: Zhu Yi <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl3945-base.c | 56 ++++++++++++++++++++++++++-
1 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 62b26be..cf41edf 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -7666,8 +7666,60 @@ static DEVICE_ATTR(power_level, S_IWUSR | S_IRUSR, show_power_level,
static ssize_t show_channels(struct device *d,
struct device_attribute *attr, char *buf)
{
- /* all this shit doesn't belong into sysfs anyway */
- return 0;
+ struct iwl3945_priv *priv = dev_get_drvdata(d);
+ struct ieee80211_channel *channels = NULL;
+ const struct ieee80211_supported_band *supp_band = NULL;
+ int len = 0, i;
+ int count = 0;
+
+ if (!test_bit(STATUS_GEO_CONFIGURED, &priv->status))
+ return -EAGAIN;
+
+ supp_band = iwl3945_get_band(priv, IEEE80211_BAND_2GHZ);
+ channels = supp_band->channels;
+ count = supp_band->n_channels;
+
+ len += sprintf(&buf[len],
+ "Displaying %d channels in 2.4GHz band "
+ "(802.11bg):\n", count);
+
+ for (i = 0; i < count; i++)
+ len += sprintf(&buf[len], "%d: %ddBm: BSS%s%s, %s.\n",
+ ieee80211_frequency_to_channel(
+ channels[i].center_freq),
+ channels[i].max_power,
+ channels[i].flags & IEEE80211_CHAN_RADAR ?
+ " (IEEE 802.11h required)" : "",
+ (!(channels[i].flags & IEEE80211_CHAN_NO_IBSS)
+ || (channels[i].flags &
+ IEEE80211_CHAN_RADAR)) ? "" :
+ ", IBSS",
+ channels[i].flags &
+ IEEE80211_CHAN_PASSIVE_SCAN ?
+ "passive only" : "active/passive");
+
+ supp_band = iwl3945_get_band(priv, IEEE80211_BAND_5GHZ);
+ channels = supp_band->channels;
+ count = supp_band->n_channels;
+
+ len += sprintf(&buf[len], "Displaying %d channels in 5.2GHz band "
+ "(802.11a):\n", count);
+
+ for (i = 0; i < count; i++)
+ len += sprintf(&buf[len], "%d: %ddBm: BSS%s%s, %s.\n",
+ ieee80211_frequency_to_channel(
+ channels[i].center_freq),
+ channels[i].max_power,
+ channels[i].flags & IEEE80211_CHAN_RADAR ?
+ " (IEEE 802.11h required)" : "",
+ ((channels[i].flags & IEEE80211_CHAN_NO_IBSS)
+ || (channels[i].flags &
+ IEEE80211_CHAN_RADAR)) ? "" :
+ ", IBSS",
+ channels[i].flags &
+ IEEE80211_CHAN_PASSIVE_SCAN ?
+ "passive only" : "active/passive");
+ return len;
}

static DEVICE_ATTR(channels, S_IRUSR, show_channels, NULL);
--
1.5.3.6



2008-09-26 05:40:17

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Thu, Sep 25, 2008 at 10:39 PM, Luis R. Rodriguez <[email protected]> wrote:
> On Thu, Sep 25, 2008 at 10:28 PM, Zhu Yi <[email protected]> wrote:
>
>> You brought it up. And I also want to discuss it. We do want to support
>> it. But the 3945/4965/5000 hardwares doesn't have the alpha2 information
>> in their EEPROMs (btw, 2100/2200 does support that). Instead, they have
>> a band/channel table including freq, tx power, etc in the EEPROM. So a
>> simple regulatory_hint() API doesn't work. We need a way to export the
>> channel details to CRDA.
>
> Yes, this detail of information was conveyed to me at OLS by Guy. This
> is why we decided to add support for letting the driver not pass an
> alpha2 but to just build the regulatory domain structure itself and
> pass it. I even provided an example of how to do this in the
> Documentation/networking/regulatory.txt and tested it myself.
>
> Essentially this was added with your drivers in mind :)
>
> Let me know if you run into any issues.

Oh and to be clear -- you "can" also pass an alpha2 if you guys do
figure out a way to do that. If not its OK, just set the alpha2 to
NULL.

Luis

2008-09-26 05:17:38

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Fri, 2008-09-26 at 13:13 +0800, Zhu Yi wrote:
> On Thu, 2008-09-25 at 21:13 -0700, Luis R. Rodriguez wrote:
> >
> > CONFIG_WIRELESS_OLD_REGULATORY is a temporary solution, the right
> > solution for you is to build the regulatory domain with the provided
> > API as discussed at OLS. Additionally distributions can start picking
> > up iw and crda. I can also see NetworkManager asking the kernel to set
> > the regulatory domain just as with iw, based on the user's defined
> > country somewhere.
>
> This is a known bug. Please read
> http://marc.info/?l=linux-wireless&m=122237262931451&w=2 and revert the
> corresponding patch.

Oops, send to the wrong address. Please ignore.

Thanks,
-yi


2008-09-25 07:34:57

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Wed, 2008-09-24 at 02:44 -0600, Johannes Berg wrote:
> On Wed, 2008-09-24 at 13:57 +0800, Zhu Yi wrote:
> > From: Abhijeet Kolekar <[email protected]>
> >
> > The patch adds channels sysfs entry for iwl3945. Make it consistent
> > with iwlagn.
>
> I advocate just make it consistent by removing it entirely.

We need a way to show the channel info the card EEPROM advocated, not
the final one intersected with the regdomain. Are you suggesting a
debugfs entry?

Thanks,
-yi


2008-09-24 08:45:29

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Wed, 2008-09-24 at 13:57 +0800, Zhu Yi wrote:
> From: Abhijeet Kolekar <[email protected]>
>
> The patch adds channels sysfs entry for iwl3945. Make it consistent
> with iwlagn.

I advocate just make it consistent by removing it entirely.

> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -7666,8 +7666,60 @@ static DEVICE_ATTR(power_level, S_IWUSR | S_IRUSR, show_power_level,
> static ssize_t show_channels(struct device *d,
> struct device_attribute *attr, char *buf)
> {
> - /* all this shit doesn't belong into sysfs anyway */

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-26 05:45:10

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Thu, 2008-09-25 at 23:39 -0600, Luis R. Rodriguez wrote:
>
> Yes, this detail of information was conveyed to me at OLS by Guy. This
> is why we decided to add support for letting the driver not pass an
> alpha2 but to just build the regulatory domain structure itself and
> pass it. I even provided an example of how to do this in the
> Documentation/networking/regulatory.txt and tested it myself.
>
> Essentially this was added with your drivers in mind :)

Thanks. We will work on this.

-yi


2008-09-26 05:28:48

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Thu, 2008-09-25 at 21:13 -0700, Luis R. Rodriguez wrote:
> CONFIG_WIRELESS_OLD_REGULATORY is a temporary solution, the right
> solution for you is to build the regulatory domain with the provided
> API as discussed at OLS. Additionally distributions can start picking
> up iw and crda. I can also see NetworkManager asking the kernel to set
> the regulatory domain just as with iw, based on the user's defined
> country somewhere.

You brought it up. And I also want to discuss it. We do want to support
it. But the 3945/4965/5000 hardwares doesn't have the alpha2 information
in their EEPROMs (btw, 2100/2200 does support that). Instead, they have
a band/channel table including freq, tx power, etc in the EEPROM. So a
simple regulatory_hint() API doesn't work. We need a way to export the
channel details to CRDA.

Thanks,
-yi


2008-09-25 11:04:12

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Thu, Sep 25, 2008 at 12:23 PM, Johannes Berg
<[email protected]> wrote:
> On Thu, 2008-09-25 at 15:34 +0800, Zhu Yi wrote:
>> On Wed, 2008-09-24 at 02:44 -0600, Johannes Berg wrote:
>> > On Wed, 2008-09-24 at 13:57 +0800, Zhu Yi wrote:
>> > > From: Abhijeet Kolekar <[email protected]>
>> > >
>> > > The patch adds channels sysfs entry for iwl3945. Make it consistent
>> > > with iwlagn.
>> >
>> > I advocate just make it consistent by removing it entirely.
>>
>> We need a way to show the channel info the card EEPROM advocated, not
>> the final one intersected with the regdomain. Are you suggesting a
>> debugfs entry?
>
> Not that I understand why you need that since iw shows all channels you
> are registering (albeit with possibly a few more flags like "disabled"),
> but yeah, if anything all this stuff should be in debugfs. There's also
> this little fact that this channel list shouldn't ever get used, you
> should be (and afaik are since I fixed it) using what the regdomain is
> enforcing, and you're already printing the channel list when enough
> debugging flags are set on driver load.



> Therefore, I don't understand why you need to clutter the driver with
> this code.

You are too development oriented not everybody is looking into kernel
logs. It's easier to ask user
to cat sysfs/debubgfs. This is not tracing this is just check static
information.

Malicious gossip has it you're trying to obfuscate the code
> to a point where everybody else refuses to work on it.

We have probably most documented HW registers from all drivers so I
doubt that has any ground :)

> Anyhow, I see none of the sysfs files you're exporting as being
> userspace API that is actually needed in that form.
>
> Also, looking into more details, your sysfs files seem to fall into
> three categories:

Most of the sysfs files are rather historic remainders, it was just
not highest priority so far to clean it up.

> 1) things that mac80211 provides but you haven't hooked up
> 2) things that are generally useful, but you've added to your driver
> instead of helping build the generic framework
> 3) things that are truly only useful for your internal debugging
>
> 1) notably TX power;

We are definitely hooked to TX power, maybe you didn't notice

> 2) it looks like retry_rate should be settable through a generic rate
> control API if it is truly useful (otherwise category 3)
This one can be removed.

power_level
> also belongs here, sometimes one can't avoid the impression that
> you're adding things like that to avoid having to help define and
> implement the generic functionality;

The patches are no the way, actually I've sent one yesterday and more
are on the way.

> 3) Neither flags nor filter flags seem useful (and filter flags you can
> actually influence via nl80211) to set outside test environments,
> statistics falls here too, same probably with antenna, status, error
> and events logs and "measurement".

These are purely for debugging and should be removed or moved to
debugfs, except log level
which I want always visible and has proven to be most useful.

> Similar issues surround the module parameters, I won't go into them
> other than saying that they constitute API as well and at least having
> different versions for 4965 and 5000 hw seems entirely pointless.
It's not pointless if you have both system on the board. This issue is
not new and
there were few attempts to add per device module parameters somehow
the patches were never accepted
into kernel.

Many
> of them seem rather pointless in general, and something not only you
> have been doing and that I've asked for before is that I'd like to see a

> generic option to disable hardware crypto in _mac80211_ rather than each
> driver because this is actually genuinely useful if you want to monitor
> frames at the same time.
This is good point also disabling and enabling hw scan I would like to
remove from module params.
This is on TODO list.

Thanks
Tomas

2008-09-26 05:14:41

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Thu, 2008-09-25 at 21:13 -0700, Luis R. Rodriguez wrote:
>
> CONFIG_WIRELESS_OLD_REGULATORY is a temporary solution, the right
> solution for you is to build the regulatory domain with the provided
> API as discussed at OLS. Additionally distributions can start picking
> up iw and crda. I can also see NetworkManager asking the kernel to set
> the regulatory domain just as with iw, based on the user's defined
> country somewhere.

This is a known bug. Please read
http://marc.info/?l=linux-wireless&m=122237262931451&w=2 and revert the
corresponding patch.

Thanks,
-yi


2008-09-25 09:23:49

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Thu, 2008-09-25 at 15:34 +0800, Zhu Yi wrote:
> On Wed, 2008-09-24 at 02:44 -0600, Johannes Berg wrote:
> > On Wed, 2008-09-24 at 13:57 +0800, Zhu Yi wrote:
> > > From: Abhijeet Kolekar <[email protected]>
> > >
> > > The patch adds channels sysfs entry for iwl3945. Make it consistent
> > > with iwlagn.
> >
> > I advocate just make it consistent by removing it entirely.
>
> We need a way to show the channel info the card EEPROM advocated, not
> the final one intersected with the regdomain. Are you suggesting a
> debugfs entry?

Not that I understand why you need that since iw shows all channels you
are registering (albeit with possibly a few more flags like "disabled"),
but yeah, if anything all this stuff should be in debugfs. There's also
this little fact that this channel list shouldn't ever get used, you
should be (and afaik are since I fixed it) using what the regdomain is
enforcing, and you're already printing the channel list when enough
debugging flags are set on driver load.

Therefore, I don't understand why you need to clutter the driver with
this code. Malicious gossip has it you're trying to obfuscate the code
to a point where everybody else refuses to work on it.

Anyhow, I see none of the sysfs files you're exporting as being
userspace API that is actually needed in that form.

Also, looking into more details, your sysfs files seem to fall into
three categories:

1) things that mac80211 provides but you haven't hooked up
2) things that are generally useful, but you've added to your driver
instead of helping build the generic framework
3) things that are truly only useful for your internal debugging

1) notably TX power;

2) it looks like retry_rate should be settable through a generic rate
control API if it is truly useful (otherwise category 3), power_level
also belongs here, sometimes one can't avoid the impression that
you're adding things like that to avoid having to help define and
implement the generic functionality;

3) Neither flags nor filter flags seem useful (and filter flags you can
actually influence via nl80211) to set outside test environments,
statistics falls here too, same probably with antenna, status, error
and events logs and "measurement".

Similar issues surround the module parameters, I won't go into them
other than saying that they constitute API as well and at least having
different versions for 4965 and 5000 hw seems entirely pointless. Many
of them seem rather pointless in general, and something not only you
have been doing and that I've asked for before is that I'd like to see a
generic option to disable hardware crypto in _mac80211_ rather than each
driver because this is actually genuinely useful if you want to monitor
frames at the same time.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-24 10:54:06

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Wed, 2008-09-24 at 10:44 +0200, Johannes Berg wrote:
> On Wed, 2008-09-24 at 13:57 +0800, Zhu Yi wrote:
> > From: Abhijeet Kolekar <[email protected]>
> >
> > The patch adds channels sysfs entry for iwl3945. Make it consistent
> > with iwlagn.
>
> I advocate just make it consistent by removing it entirely.
>
> > --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> > @@ -7666,8 +7666,60 @@ static DEVICE_ATTR(power_level, S_IWUSR | S_IRUSR, show_power_level,
> > static ssize_t show_channels(struct device *d,
> > struct device_attribute *attr, char *buf)
> > {
> > - /* all this shit doesn't belong into sysfs anyway */

Yeah, it seems odd.

Dan



2008-09-25 11:50:45

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

>
> Didn't see it on iwl3945, but I only looked for the function that the
> sysfs code calls and didn't find any other callsite, might well be a
> different way.

Might be case for 3945 I'm less involved in that one.


>
>> except log level
>> which I want always visible and has proven to be most useful.
>
> Yeah, indeed, though it seems to be notably lacking printing out the
> physical addresses and the ringbuffer entries. But that's just the thing
> I was looking for yesterday when debugging the corruption issue.

I should have patches for that somewhere got lost somewhere in
selective merging, will bring them forward.

>> > generic option to disable hardware crypto in _mac80211_ rather than each
>> > driver because this is actually genuinely useful if you want to monitor
>> > frames at the same time.
>
>> This is good point also disabling and enabling hw scan I would like to
>> remove from module params.
>
> That would be doable via mac80211 too, though that is a much more debug
> option than disabling hardware crypto, I see disabling hw crypto as much
> more useful than disabling hw scan.

I would probably remove the option of disabling hw scan at all, think
it's stable enough.
Tomas

2008-09-26 05:39:29

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Thu, Sep 25, 2008 at 10:28 PM, Zhu Yi <[email protected]> wrote:

> You brought it up. And I also want to discuss it. We do want to support
> it. But the 3945/4965/5000 hardwares doesn't have the alpha2 information
> in their EEPROMs (btw, 2100/2200 does support that). Instead, they have
> a band/channel table including freq, tx power, etc in the EEPROM. So a
> simple regulatory_hint() API doesn't work. We need a way to export the
> channel details to CRDA.

Yes, this detail of information was conveyed to me at OLS by Guy. This
is why we decided to add support for letting the driver not pass an
alpha2 but to just build the regulatory domain structure itself and
pass it. I even provided an example of how to do this in the
Documentation/networking/regulatory.txt and tested it myself.

Essentially this was added with your drivers in mind :)

Let me know if you run into any issues.

Luis

2008-09-26 03:54:46

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Thu, 2008-09-25 at 03:23 -0600, Johannes Berg wrote:
> Not that I understand why you need that since iw shows all channels
> you
> are registering (albeit with possibly a few more flags like
> "disabled"),
> but yeah, if anything all this stuff should be in debugfs.

The real user for the function is the automated test cases of our
validation team running nightly. Greppig dmesg is not reliable and iw is
not mature enough (at least not included in major distros). We will move
it to debugfs for now. After iw is as common as iwconfig, we will
eventually remove this.

> There's also
> this little fact that this channel list shouldn't ever get used, you
> should be (and afaik are since I fixed it) using what the regdomain is
> enforcing, and you're already printing the channel list when enough
> debugging flags are set on driver load.

This is used for debugging especially during the time regdomain is under
development. I've received quite a few bug reports that is due to
CONFIG_WIRELESS_OLD_REGULATORY is not enabled. I need at least a way to
tell the users this is due to your misconfig your regdomain or your
hardware is not capable to work on that channel.

Thanks,
-yi


2008-09-24 06:02:28

by Zhu Yi

[permalink] [raw]
Subject: [PATCH] iwlwifi: don't fail if scan is issued too early

From: Tomas Winkler <[email protected]>

This patch returns success and empty scan on scans requests that were
rejected because issued too early. The cached bss list from previous
scanning will be returned by mac80211.

Signed-off-by: Tomas Winkler <[email protected]>
Signed-off-by: Zhu Yi <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-agn.c | 15 +++++++++++----
drivers/net/wireless/iwlwifi/iwl-scan.c | 8 +-------
2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index c31adf6..e6d742f 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -3190,9 +3190,9 @@ static void iwl4965_bss_info_changed(struct ieee80211_hw *hw,

static int iwl_mac_hw_scan(struct ieee80211_hw *hw, u8 *ssid, size_t ssid_len)
{
- int ret;
unsigned long flags;
struct iwl_priv *priv = hw->priv;
+ int ret;

IWL_DEBUG_MAC80211("enter\n");

@@ -3211,20 +3211,27 @@ static int iwl_mac_hw_scan(struct ieee80211_hw *hw, u8 *ssid, size_t ssid_len)
goto out_unlock;
}

- /* we don't schedule scan within next_scan_jiffies period */
+ /* We don't schedule scan within next_scan_jiffies period.
+ * Avoid scanning during possible EAPOL exchange, return
+ * success immediately.
+ */
if (priv->next_scan_jiffies &&
time_after(priv->next_scan_jiffies, jiffies)) {
IWL_DEBUG_SCAN("scan rejected: within next scan period\n");
- ret = -EAGAIN;
+ queue_work(priv->workqueue, &priv->scan_completed);
+ ret = 0;
goto out_unlock;
}
+
/* if we just finished scan ask for delay */
if (iwl_is_associated(priv) && priv->last_scan_jiffies &&
time_after(priv->last_scan_jiffies + IWL_DELAY_NEXT_SCAN, jiffies)) {
IWL_DEBUG_SCAN("scan rejected: within previous scan period\n");
- ret = -EAGAIN;
+ queue_work(priv->workqueue, &priv->scan_completed);
+ ret = 0;
goto out_unlock;
}
+
if (ssid_len) {
priv->one_direct_scan = 1;
priv->direct_ssid_len = min_t(u8, ssid_len, IW_ESSID_MAX_SIZE);
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 09c264b..bf855c3 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -463,11 +463,6 @@ void iwl_init_scan_params(struct iwl_priv *priv)

int iwl_scan_initiate(struct iwl_priv *priv)
{
- if (priv->iw_mode == NL80211_IFTYPE_AP) {
- IWL_ERROR("APs don't scan.\n");
- return 0;
- }
-
if (!iwl_is_ready_rf(priv)) {
IWL_DEBUG_SCAN("Aborting scan due to not ready.\n");
return -EIO;
@@ -479,8 +474,7 @@ int iwl_scan_initiate(struct iwl_priv *priv)
}

if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
- IWL_DEBUG_SCAN("Scan request while abort pending. "
- "Queuing.\n");
+ IWL_DEBUG_SCAN("Scan request while abort pending\n");
return -EAGAIN;
}

--
1.5.3.6


2008-09-25 11:21:42

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Thu, 2008-09-25 at 14:04 +0300, Tomas Winkler wrote:

> > Therefore, I don't understand why you need to clutter the driver with
> > this code.
>
> You are too development oriented not everybody is looking into kernel
> logs. It's easier to ask user
> to cat sysfs/debubgfs. This is not tracing this is just check static
> information.

Shouldn't be much harder to ask users to provide kernel logs, we've been
doing that with b43 forever.

> Malicious gossip has it you're trying to obfuscate the code
> > to a point where everybody else refuses to work on it.
>
> We have probably most documented HW registers from all drivers so I
> doubt that has any ground :)

Well, in the code, I'm sure b43 has more documentation, I wrote it ;)

> We are definitely hooked to TX power, maybe you didn't notice

Didn't see it on iwl3945, but I only looked for the function that the
sysfs code calls and didn't find any other callsite, might well be a
different way.

> power_level
> > also belongs here, sometimes one can't avoid the impression that
> > you're adding things like that to avoid having to help define and
> > implement the generic functionality;
>
> The patches are no the way, actually I've sent one yesterday and more
> are on the way.

yay :)

> except log level
> which I want always visible and has proven to be most useful.

Yeah, indeed, though it seems to be notably lacking printing out the
physical addresses and the ringbuffer entries. But that's just the thing
I was looking for yesterday when debugging the corruption issue.

> > generic option to disable hardware crypto in _mac80211_ rather than each
> > driver because this is actually genuinely useful if you want to monitor
> > frames at the same time.

> This is good point also disabling and enabling hw scan I would like to
> remove from module params.

That would be doable via mac80211 too, though that is a much more debug
option than disabling hardware crypto, I see disabling hw crypto as much
more useful than disabling hw scan.

> This is on TODO list.

:)

Thanks,
johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-26 04:13:30

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] iwl3945: added channel sysfs entry

On Thu, Sep 25, 2008 at 8:54 PM, Zhu Yi <[email protected]> wrote:
> On Thu, 2008-09-25 at 03:23 -0600, Johannes Berg wrote:
>> Not that I understand why you need that since iw shows all channels
>> you
>> are registering (albeit with possibly a few more flags like
>> "disabled"),
>> but yeah, if anything all this stuff should be in debugfs.
>
> The real user for the function is the automated test cases of our
> validation team running nightly. Greppig dmesg is not reliable and iw is
> not mature enough (at least not included in major distros). We will move
> it to debugfs for now. After iw is as common as iwconfig, we will
> eventually remove this.

What a distribution carries has no implications as to what you can
install on your own environment. We have both iw and crda installed in
our test environment, but that's also because we like to focus on
wireless-testing.

If you do development based on 2 previous releases essentially you
will get complaints on the type of patches you post based on this old
stuff.

>> There's also
>> this little fact that this channel list shouldn't ever get used, you
>> should be (and afaik are since I fixed it) using what the regdomain is
>> enforcing, and you're already printing the channel list when enough
>> debugging flags are set on driver load.
>
> This is used for debugging especially during the time regdomain is under
> development. I've received quite a few bug reports that is due to
> CONFIG_WIRELESS_OLD_REGULATORY is not enabled. I need at least a way to
> tell the users this is due to your misconfig your regdomain or your
> hardware is not capable to work on that channel.

CONFIG_WIRELESS_OLD_REGULATORY is a temporary solution, the right
solution for you is to build the regulatory domain with the provided
API as discussed at OLS. Additionally distributions can start picking
up iw and crda. I can also see NetworkManager asking the kernel to set
the regulatory domain just as with iw, based on the user's defined
country somewhere.

Luis