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

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

[Bjørn Mork]

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 <bjorn@mork.no>
---
 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

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

[reinette chatre]

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 <jay.e.sternberg@intel.com>
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 <jay.e.sternberg@intel.com>
    Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
    Signed-off-by: John W. Linville <linville@tuxdriver.com>

Reinette

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

[Bjørn Mork]

reinette chatre <reinette.chatre@intel.com> 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 <reinette.chatre@intel.com>
Date:   Thu Jul 9 10:33:37 2009 -0700

    iwlwifi: only show active power level via sysfs
    


Bjørn

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

[reinette chatre]

Hi Bjørn,

On Thu, 2009-08-27 at 08:09 -0700, Bjørn Mork wrote:
> reinette chatre <reinette.chatre@intel.com> 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

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

[Bjørn Mork]

reinette chatre <reinette.chatre@intel.com> 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