| Message ID | 1365075480-20183-6-git-send-email-jeffrey.t.kirsher@intel.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, 2013-04-04 at 04:37 -0700, Jeff Kirsher wrote: > From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com> > > This patch adds support to read and export SFF-8472/8079 (SFP data) > over i2c, through Ethtool. [...] > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c [...] > +static int igb_get_module_eeprom(struct net_device *netdev, > + struct ethtool_eeprom *ee, u8 *data) > +{ > + struct igb_adapter *adapter = netdev_priv(netdev); > + struct e1000_hw *hw = &adapter->hw; > + u32 status = E1000_SUCCESS; > + u16 dataword = 0xFF; > + int i = 0; > + > + /* Read the first block, SFF-8079, 2 bytes at a time */ > + for (i = 0; i < ETH_MODULE_SFF_8079_LEN; i += 2) { > + status = igb_read_phy_reg_i2c(hw, i, &dataword); > + if (status != E1000_SUCCESS) > + /* Error occurred while reading module */ > + return -EIO; > + > + data[i] = (dataword >> 8) & 0xFF; > + data[i+1] = dataword & 0xFF; > + } What if ee->offset != 0 or ee->len < ETH_MODULE_SFF_8079_LEN? > + /* If the second block is requested, check if SFF-8472 is supported. */ > + if (ee->len == ETH_MODULE_SFF_8472_LEN) { > + if (data[IGB_SFF_8472_COMP] == IGB_SFF_8472_UNSUP) > + return -EOPNOTSUPP; > + > + /* Read the second block, SFF-8472, 2 bytes at a time */ > + for (i = ETH_MODULE_SFF_8079_LEN; > + i < ETH_MODULE_SFF_8472_LEN; i += 2) { > + status = igb_read_phy_reg_i2c(hw, i, &dataword); > + if (status != E1000_SUCCESS) > + return -EIO; > + > + data[i] = (dataword >> 8) & 0xFF; > + data[i+1] = dataword & 0xFF; > + } > + } [...] What if ee->offset != 0 or ETH_MODULE_SFF_8079_LEN < ee->len < ETH_MODULE_SFF_8472_LEN? Ben.
> -----Original Message----- > From: Ben Hutchings [mailto:bhutchings@solarflare.com] > Sent: Thursday, April 04, 2013 12:08 PM > To: Kirsher, Jeffrey T > Cc: davem@davemloft.net; Abodunrin, Akeem G; netdev@vger.kernel.org; > gospo@redhat.com; sassmann@redhat.com; Aurélien Guillaume > Subject: Re: [net-next 05/13] igb: Support to read and export SFF-8472/8079 > data > > On Thu, 2013-04-04 at 04:37 -0700, Jeff Kirsher wrote: > > From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com> > > > > This patch adds support to read and export SFF-8472/8079 (SFP data) > > over i2c, through Ethtool. > [...] > > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c > > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c > [...] > > +static int igb_get_module_eeprom(struct net_device *netdev, > > + struct ethtool_eeprom *ee, u8 *data) { > > + struct igb_adapter *adapter = netdev_priv(netdev); > > + struct e1000_hw *hw = &adapter->hw; > > + u32 status = E1000_SUCCESS; > > + u16 dataword = 0xFF; > > + int i = 0; > > + > > + /* Read the first block, SFF-8079, 2 bytes at a time */ > > + for (i = 0; i < ETH_MODULE_SFF_8079_LEN; i += 2) { > > + status = igb_read_phy_reg_i2c(hw, i, &dataword); > > + if (status != E1000_SUCCESS) > > + /* Error occurred while reading module */ > > + return -EIO; > > + > > + data[i] = (dataword >> 8) & 0xFF; > > + data[i+1] = dataword & 0xFF; > > + } > > What if ee->offset != 0 or ee->len < ETH_MODULE_SFF_8079_LEN? > > > + /* If the second block is requested, check if SFF-8472 is supported. */ > > + if (ee->len == ETH_MODULE_SFF_8472_LEN) { > > + if (data[IGB_SFF_8472_COMP] == IGB_SFF_8472_UNSUP) > > + return -EOPNOTSUPP; > > + > > + /* Read the second block, SFF-8472, 2 bytes at a time */ > > + for (i = ETH_MODULE_SFF_8079_LEN; > > + i < ETH_MODULE_SFF_8472_LEN; i += 2) { > > + status = igb_read_phy_reg_i2c(hw, i, &dataword); > > + if (status != E1000_SUCCESS) > > + return -EIO; > > + > > + data[i] = (dataword >> 8) & 0xFF; > > + data[i+1] = dataword & 0xFF; > > + } > > + } > [...] > > What if ee->offset != 0 or ETH_MODULE_SFF_8079_LEN < ee->len < > ETH_MODULE_SFF_8472_LEN? Thanks Ben, Yes, you are right... I believe this is an oversight. There may be need to use ee->offset as starting point and make sure ETH_MODULE_SFF_8079_LEN or ETH_MODULE_SFF_8472_LEN is within ee->len scope. We would address this in the follow up patch. Regards, ~Akeem > > 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/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h index 2515140..7cb0398 100644 --- a/drivers/net/ethernet/intel/igb/igb.h +++ b/drivers/net/ethernet/intel/igb/igb.h @@ -178,6 +178,14 @@ enum igb_tx_flags { #define TXD_USE_COUNT(S) DIV_ROUND_UP((S), IGB_MAX_DATA_PER_TXD) #define DESC_NEEDED (MAX_SKB_FRAGS + 4) +/* EEPROM byte offsets */ +#define IGB_SFF_8472_SWAP 0x5C +#define IGB_SFF_8472_COMP 0x5E + +/* Bitmasks */ +#define IGB_SFF_ADDRESSING_MODE 0x4 +#define IGB_SFF_8472_UNSUP 0x00 + /* wrapper around a pointer to a socket buffer, * so a DMA handle can be stored along with the buffer */ struct igb_tx_buffer { diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c index 8499c48..17443e4 100644 --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c @@ -2622,6 +2622,88 @@ static int igb_set_eee(struct net_device *netdev, return 0; } +static int igb_get_module_info(struct net_device *netdev, + struct ethtool_modinfo *modinfo) +{ + struct igb_adapter *adapter = netdev_priv(netdev); + struct e1000_hw *hw = &adapter->hw; + u32 status = E1000_SUCCESS; + u16 sff8472_rev, addr_mode; + bool page_swap = false; + + if ((hw->phy.media_type == e1000_media_type_copper) || + (hw->phy.media_type == e1000_media_type_unknown)) + return -EOPNOTSUPP; + + /* Check whether we support SFF-8472 or not */ + status = igb_read_phy_reg_i2c(hw, IGB_SFF_8472_COMP, &sff8472_rev); + if (status != E1000_SUCCESS) + return -EIO; + + /* addressing mode is not supported */ + status = igb_read_phy_reg_i2c(hw, IGB_SFF_8472_SWAP, &addr_mode); + if (status != E1000_SUCCESS) + return -EIO; + + /* addressing mode is not supported */ + if ((addr_mode & 0xFF) & IGB_SFF_ADDRESSING_MODE) { + hw_dbg("Address change required to access page 0xA2, but not supported. Please report the module type to the driver maintainers.\n"); + page_swap = true; + } + + if ((sff8472_rev & 0xFF) == IGB_SFF_8472_UNSUP || page_swap) { + /* We have an SFP, but it does not support SFF-8472 */ + modinfo->type = ETH_MODULE_SFF_8079; + modinfo->eeprom_len = ETH_MODULE_SFF_8079_LEN; + } else { + /* We have an SFP which supports a revision of SFF-8472 */ + modinfo->type = ETH_MODULE_SFF_8472; + modinfo->eeprom_len = ETH_MODULE_SFF_8472_LEN; + } + + return 0; +} + +static int igb_get_module_eeprom(struct net_device *netdev, + struct ethtool_eeprom *ee, u8 *data) +{ + struct igb_adapter *adapter = netdev_priv(netdev); + struct e1000_hw *hw = &adapter->hw; + u32 status = E1000_SUCCESS; + u16 dataword = 0xFF; + int i = 0; + + /* Read the first block, SFF-8079, 2 bytes at a time */ + for (i = 0; i < ETH_MODULE_SFF_8079_LEN; i += 2) { + status = igb_read_phy_reg_i2c(hw, i, &dataword); + if (status != E1000_SUCCESS) + /* Error occurred while reading module */ + return -EIO; + + data[i] = (dataword >> 8) & 0xFF; + data[i+1] = dataword & 0xFF; + } + + /* If the second block is requested, check if SFF-8472 is supported. */ + if (ee->len == ETH_MODULE_SFF_8472_LEN) { + if (data[IGB_SFF_8472_COMP] == IGB_SFF_8472_UNSUP) + return -EOPNOTSUPP; + + /* Read the second block, SFF-8472, 2 bytes at a time */ + for (i = ETH_MODULE_SFF_8079_LEN; + i < ETH_MODULE_SFF_8472_LEN; i += 2) { + status = igb_read_phy_reg_i2c(hw, i, &dataword); + if (status != E1000_SUCCESS) + return -EIO; + + data[i] = (dataword >> 8) & 0xFF; + data[i+1] = dataword & 0xFF; + } + } + + return 0; +} + static int igb_ethtool_begin(struct net_device *netdev) { struct igb_adapter *adapter = netdev_priv(netdev); @@ -2666,6 +2748,8 @@ static const struct ethtool_ops igb_ethtool_ops = { .set_rxnfc = igb_set_rxnfc, .get_eee = igb_get_eee, .set_eee = igb_set_eee, + .get_module_info = igb_get_module_info, + .get_module_eeprom = igb_get_module_eeprom, .begin = igb_ethtool_begin, .complete = igb_ethtool_complete, };