2009-08-27 13:43:29

by Bjørn Mork

[permalink] [raw]
Subject: [PATCH] iwlagn: show_version() displays confusing/wrong firmware version

The output of show_version() is confusing at best, and can also be
considered wrong if you don't know that the order of the API and
SERIAL number has been switched. The unusual dotted hex is also
unecessary unreadable and different from the filename convention and
outout from iwl_read_ucode():

bjorn@nemi:~$ cat /sys/class/net/wlan0/device/version
fw version: 0x8.0x18.0xC.0x2
fw type: 0x1 0x0
EEPROM version: 0x11e

iwl_read_ucode() prints this when loading the same firmware:

[ 21.406218] iwlagn 0000:03:00.0: firmware: requesting iwlwifi-5000-2.ucode
[ 21.453118] iwlagn 0000:03:00.0: loaded firmware version 8.24.2.12

Note that I have no documentation on the intended usage of the
u8 sw_rev[8] array in struct iwl_alive_resp. sw_rev[0] and sw_rev[1]
have been switched to make the output match iwl_read_ucode(). Nothing
more, nothing less.

Signed-off-by: Bjørn Mork <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-agn.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 355f50e..1a1ccb4 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -2498,10 +2498,10 @@ static ssize_t show_version(struct device *d,

if (palive->is_valid)
pos += sprintf(buf + pos,
- "fw version: 0x%01X.0x%01X.0x%01X.0x%01X\n"
+ "fw version: %u.%u.%u.%u\n"
"fw type: 0x%01X 0x%01X\n",
palive->ucode_major, palive->ucode_minor,
- palive->sw_rev[0], palive->sw_rev[1],
+ palive->sw_rev[1], palive->sw_rev[0],
palive->ver_type, palive->ver_subtype);
else
pos += sprintf(buf + pos, "fw not loaded\n");
--
1.5.6.5



2009-08-27 16:50:56

by Bjørn Mork

[permalink] [raw]
Subject: Re: [PATCH] iwlagn: show_version() displays confusing/wrong firmware version

reinette chatre <[email protected]> writes:

> Since 2.6.31 is considered a done deal and this is surely not the
> required "earth shattering" fix (see [1]) I cannot push this as a fix
> into Linus's repo at this time. This display problem will not exist in
> 2.6.32.

Ah, I see. Yes, that looks even better. I feel a bit stupid for not
checking iwlwifi-2.6.git before sending this. Thanks for your patience.


Bjørn

2009-08-27 15:29:24

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH] iwlagn: show_version() displays confusing/wrong firmware version

Hi Bjørn,

On Thu, 2009-08-27 at 08:09 -0700, Bjørn Mork wrote:
> reinette chatre <[email protected]> writes:
> > Which kernel/repo is your patch based on?
>
> Oh, sorry for not including that vital information. I'm afraid it is
> against Linus' tree at
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> and not any of the wireless/networking trees.

Since 2.6.31 is considered a done deal and this is surely not the
required "earth shattering" fix (see [1]) I cannot push this as a fix
into Linus's repo at this time. This display problem will not exist in
2.6.32.

Reinette

[1] http://article.gmane.org/gmane.linux.kernel.wireless.general/38579


2009-08-27 14:57:20

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH] iwlagn: show_version() displays confusing/wrong firmware version

Hi Bjørn,

Which kernel/repo is your patch based on?

On Thu, 2009-08-27 at 06:33 -0700, Bjørn Mork wrote:
> The output of show_version() is confusing at best, and can also be
> considered wrong

Correct. Since this information is already printed in the system logs we
determined that the version sysfs file is not needed and has been
removed. Your patch is thus not relevant to the recent code
(wireless-testing repository).

Here is the patch for your reference:

commit 44f313c2e63dcf93b17e6a43769105e487e2e49d
Author: Jay Sternberg <[email protected]>
Date: Fri Jul 31 14:28:09 2009 -0700

iwlwifi: remove duplicated version info from sysfs

version info in sysfs had been determined to be unnecessary as it
is already provided in syslog info. nvm version is added to syslog
version info as a debug level message to provide all info that was
in the version sysfs data.

Signed-off-by: Jay Sternberg <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
Signed-off-by: John W. Linville <[email protected]>

Reinette



2009-08-27 15:28:29

by Bjørn Mork

[permalink] [raw]
Subject: Re: [PATCH] iwlagn: show_version() displays confusing/wrong firmware version

reinette chatre <[email protected]> writes:

> Hi Bjørn,
>
> Which kernel/repo is your patch based on?

Oh, sorry for not including that vital information. I'm afraid it is
against Linus' tree at
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
and not any of the wireless/networking trees.

"git log drivers/net/wireless/iwlwifi/iwl-agn.c" shows this as the last
commit affecting iwl-agn.c before my change:

commit 872ed1902f511a8947021c562f5728a5bf0640b5
Author: Reinette Chatre <[email protected]>
Date: Thu Jul 9 10:33:37 2009 -0700

iwlwifi: only show active power level via sysfs



Bjørn