| Message ID | 1318854062-3628-7-git-send-email-jeffrey.t.kirsher@intel.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2011-10-17 at 05:21 -0700, Jeff Kirsher wrote: > From: Emil Tantilov <emil.s.tantilov@intel.com> > > Use 32bit value starting at offset 0x2d for displaying the firmware > version in ethtool. This should work for all current ixgbe HW [] > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c [] > - snprintf(firmware_version, sizeof(firmware_version), "%d.%d-%d", > - (adapter->eeprom_version & 0xF000) >> 12, > - (adapter->eeprom_version & 0x0FF0) >> 4, > - adapter->eeprom_version & 0x000F); > + nvm_track_id = (adapter->eeprom_verh << 16) | > + adapter->eeprom_verl; > + snprintf(firmware_version, sizeof(firmware_version), "0x%08x", > + nvm_track_id); Is ethtool output like proc output considered an abi that should not be changed? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2011-10-17 at 08:57 -0700, Joe Perches wrote: > On Mon, 2011-10-17 at 05:21 -0700, Jeff Kirsher wrote: > > From: Emil Tantilov <emil.s.tantilov@intel.com> > > > > Use 32bit value starting at offset 0x2d for displaying the firmware > > version in ethtool. This should work for all current ixgbe HW > [] > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c > [] > > - snprintf(firmware_version, sizeof(firmware_version), "%d.%d-%d", > > - (adapter->eeprom_version & 0xF000) >> 12, > > - (adapter->eeprom_version & 0x0FF0) >> 4, > > - adapter->eeprom_version & 0x000F); > > + nvm_track_id = (adapter->eeprom_verh << 16) | > > + adapter->eeprom_verl; > > + snprintf(firmware_version, sizeof(firmware_version), "0x%08x", > > + nvm_track_id); > > Is ethtool output like proc output considered an abi > that should not be changed? No-one should make any assumptions about the format of firmware_version strings. However they ought to be consistent with vendor documentation, update programs, etc. Ben.
>-----Original Message----- >From: Ben Hutchings [mailto:bhutchings@solarflare.com] >Sent: Monday, October 17, 2011 10:17 AM >To: Joe Perches >Cc: Kirsher, Jeffrey T; davem@davemloft.net; Tantilov, Emil S; >netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com >Subject: Re: [net-next 6/6] ixgbe: change the eeprom version reported by >ethtool > >On Mon, 2011-10-17 at 08:57 -0700, Joe Perches wrote: >> On Mon, 2011-10-17 at 05:21 -0700, Jeff Kirsher wrote: >> > From: Emil Tantilov <emil.s.tantilov@intel.com> >> > >> > Use 32bit value starting at offset 0x2d for displaying the firmware >> > version in ethtool. This should work for all current ixgbe HW >> [] >> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c >b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c >> [] >> > - snprintf(firmware_version, sizeof(firmware_version), "%d.%d-%d", >> > - (adapter->eeprom_version & 0xF000) >> 12, >> > - (adapter->eeprom_version & 0x0FF0) >> 4, >> > - adapter->eeprom_version & 0x000F); >> > + nvm_track_id = (adapter->eeprom_verh << 16) | >> > + adapter->eeprom_verl; >> > + snprintf(firmware_version, sizeof(firmware_version), "0x%08x", >> > + nvm_track_id); >> >> Is ethtool output like proc output considered an abi >> that should not be changed? > >No-one should make any assumptions about the format of firmware_version >strings. However they ought to be consistent with vendor documentation, >update programs, etc. The old value was only marginally useful as it was possible to have different images with the same version. The 32 bit value shown by this patch is what is being used by the FW team to record the revisions of the images. The words used to hold the 32 bit value are not described in the datasheet for 82598/9, but will be in X540 once it becomes available and I have requested that this information be added to the 82598/9 docs as well. Thanks, Emil > >Ben. > >-- >Ben Hutchings, Staff Engineer, Solarflare >Not speaking for my employer; that's the marketing department's job. >They asked us to note that Solarflare product names are trademarked.
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h index 9e6635e..4881807 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h @@ -518,7 +518,8 @@ struct ixgbe_adapter { u64 rsc_total_count; u64 rsc_total_flush; u32 wol; - u16 eeprom_version; + u16 eeprom_verh; + u16 eeprom_verl; u16 eeprom_cap; int node; diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c index 7acfce3..70d58c3 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c @@ -889,21 +889,22 @@ static void ixgbe_get_drvinfo(struct net_device *netdev, { struct ixgbe_adapter *adapter = netdev_priv(netdev); char firmware_version[32]; + u32 nvm_track_id; strncpy(drvinfo->driver, ixgbe_driver_name, sizeof(drvinfo->driver) - 1); strncpy(drvinfo->version, ixgbe_driver_version, sizeof(drvinfo->version) - 1); - snprintf(firmware_version, sizeof(firmware_version), "%d.%d-%d", - (adapter->eeprom_version & 0xF000) >> 12, - (adapter->eeprom_version & 0x0FF0) >> 4, - adapter->eeprom_version & 0x000F); + nvm_track_id = (adapter->eeprom_verh << 16) | + adapter->eeprom_verl; + snprintf(firmware_version, sizeof(firmware_version), "0x%08x", + nvm_track_id); strncpy(drvinfo->fw_version, firmware_version, - sizeof(drvinfo->fw_version)); + sizeof(drvinfo->fw_version) - 1); strncpy(drvinfo->bus_info, pci_name(adapter->pdev), - sizeof(drvinfo->bus_info)); + sizeof(drvinfo->bus_info) - 1); drvinfo->n_stats = IXGBE_STATS_LEN; drvinfo->testinfo_len = IXGBE_TEST_LEN; drvinfo->regdump_len = ixgbe_get_regs_len(netdev); diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index c92c3a7..31f4f53 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -8066,6 +8066,10 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev, } device_set_wakeup_enable(&adapter->pdev->dev, adapter->wol); + /* save off EEPROM version number */ + hw->eeprom.ops.read(hw, 0x2e, &adapter->eeprom_verh); + hw->eeprom.ops.read(hw, 0x2d, &adapter->eeprom_verl); + /* pick up the PCI bus settings for reporting later */ hw->mac.ops.get_bus_info(hw); @@ -8115,9 +8119,6 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev, "is required.\n"); } - /* save off EEPROM version number */ - hw->eeprom.ops.read(hw, 0x29, &adapter->eeprom_version); - /* reset the hardware with the new settings */ err = hw->mac.ops.start_hw(hw);