From: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> Adding API for get_module_eeprom and get_module_info. Signed-off-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> --- doc/guides/nics/features/axgbe.ini | 1 + drivers/net/axgbe/axgbe_ethdev.c | 2 + drivers/net/axgbe/axgbe_phy.h | 4 ++ drivers/net/axgbe/axgbe_phy_impl.c | 107 +++++++++++++++++++++++++++++ 4 files changed, 114 insertions(+) diff --git a/doc/guides/nics/features/axgbe.ini b/doc/guides/nics/features/axgbe.ini index 0becaa097..80a3bcee1 100644 --- a/doc/guides/nics/features/axgbe.ini +++ b/doc/guides/nics/features/axgbe.ini @@ -18,3 +18,4 @@ Basic stats = Y Linux UIO = Y x86-32 = Y x86-64 = Y +Module EEPROM dump = Y diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c index 867058845..ea2f9bba1 100644 --- a/drivers/net/axgbe/axgbe_ethdev.c +++ b/drivers/net/axgbe/axgbe_ethdev.c @@ -214,6 +214,8 @@ static const struct eth_dev_ops axgbe_eth_dev_ops = { .dev_supported_ptypes_get = axgbe_dev_supported_ptypes_get, .rx_descriptor_status = axgbe_dev_rx_descriptor_status, .tx_descriptor_status = axgbe_dev_tx_descriptor_status, + .get_module_info = axgbe_get_module_info, + .get_module_eeprom = axgbe_get_module_eeprom, }; static int axgbe_phy_reset(struct axgbe_port *pdata) diff --git a/drivers/net/axgbe/axgbe_phy.h b/drivers/net/axgbe/axgbe_phy.h index 77ee20a31..d9d7fde41 100644 --- a/drivers/net/axgbe/axgbe_phy.h +++ b/drivers/net/axgbe/axgbe_phy.h @@ -188,5 +188,9 @@ #define DUPLEX_FULL 0x01 #define DUPLEX_UNKNOWN 0xff +int axgbe_get_module_info(struct rte_eth_dev *dev, + struct rte_eth_dev_module_info *modinfo); +int axgbe_get_module_eeprom(struct rte_eth_dev *dev, + struct rte_dev_eeprom_info *info); #endif /* PHY */ diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c index 02236ec19..53716071f 100644 --- a/drivers/net/axgbe/axgbe_phy_impl.c +++ b/drivers/net/axgbe/axgbe_phy_impl.c @@ -141,12 +141,18 @@ enum axgbe_sfp_speed { #define AXGBE_SFP_EXTD_CC 31 +#define AXGBE_SFP_EEPROM_PAGE_SIZE 256 + struct axgbe_sfp_eeprom { u8 base[64]; u8 extd[32]; u8 vendor[32]; }; +struct axgbe_sfp_eeprom_module { + u8 base[256]; +}; + #define AXGBE_BEL_FUSE_VENDOR "BEL-FUSE" #define AXGBE_BEL_FUSE_PARTNO "1GBT-SFP06" @@ -734,6 +740,106 @@ static int axgbe_phy_sfp_read_eeprom(struct axgbe_port *pdata) return ret; } +int axgbe_get_module_info(struct rte_eth_dev *dev, + struct rte_eth_dev_module_info *modinfo) +{ + struct axgbe_port *pdata = dev->data->dev_private; + struct axgbe_sfp_eeprom sfp_eeprom; + uint8_t eeprom_addr; + int ret; + + ret = axgbe_phy_get_comm_ownership(pdata); + + if (ret) + return -EIO; + + ret = axgbe_phy_sfp_get_mux(pdata); + + if (ret) { + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); + return ret; + } + + eeprom_addr = 0; + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, + &eeprom_addr, sizeof(eeprom_addr), + &sfp_eeprom, sizeof(sfp_eeprom)); + + if (ret) { + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); + goto put; + } + + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] != 0xff) { + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] == 0) { + modinfo->type = RTE_ETH_MODULE_SFF_8079; + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8079_LEN; + } else { + modinfo->type = RTE_ETH_MODULE_SFF_8472; + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8472_LEN; + } + } + + +put: + axgbe_phy_sfp_put_mux(pdata); + axgbe_phy_put_comm_ownership(pdata); + return 0; +} + +int axgbe_get_module_eeprom(struct rte_eth_dev *dev, + struct rte_dev_eeprom_info *info) +{ + struct axgbe_port *pdata = dev->data->dev_private; + struct axgbe_sfp_eeprom_module sfp_eeprom; + uint8_t eeprom_addr; + uint8_t *data; + uint32_t i; + int ret; + + ret = axgbe_phy_get_comm_ownership(pdata); + + if (ret) + return -EIO; + + if (!info || !info->length || !info->data) + return -EINVAL; + + ret = axgbe_phy_sfp_get_mux(pdata); + if (ret) { + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); + return ret; + } + + eeprom_addr = 0; + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, + &eeprom_addr, sizeof(eeprom_addr), + &sfp_eeprom, sizeof(sfp_eeprom)); + if (ret) { + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); + goto put; + } + data = info->data; + + /* for AXGBE_SFP_SERIAL_ID_ADDRESS */ + for (i = 0; i < AXGBE_SFP_EEPROM_PAGE_SIZE; i++) + data[i] = sfp_eeprom.base[i]; + + eeprom_addr = 0; + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_DIAG_INFO_ADDRESS, + &eeprom_addr, sizeof(eeprom_addr), + &sfp_eeprom, sizeof(sfp_eeprom)); + + /* for AXGBE_SFP_DIAG_INFO_ADDRESS */ + for (i = 0; i < info->length - AXGBE_SFP_EEPROM_PAGE_SIZE; i++) + data[i + AXGBE_SFP_EEPROM_PAGE_SIZE] = sfp_eeprom.base[i]; + +put: + axgbe_phy_sfp_put_mux(pdata); + axgbe_phy_put_comm_ownership(pdata); + return 0; +} + static void axgbe_phy_sfp_signals(struct axgbe_port *pdata) { struct axgbe_phy_data *phy_data = pdata->phy_data; @@ -741,6 +847,7 @@ static void axgbe_phy_sfp_signals(struct axgbe_port *pdata) u8 gpio_reg, gpio_ports[2]; int ret; + /* Read the input port registers */ gpio_reg = 0; ret = axgbe_phy_i2c_read(pdata, phy_data->sfp_gpio_address, -- 2.17.1
On 5/14/2020 2:43 PM, asomalap@amd.com wrote: > From: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> > > Adding API for get_module_eeprom and get_module_info. > > Signed-off-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> Hi Amaranath, I guess this is v3 of the patch, can you please use the version info and keep the all versions in same thread (using --in-reply-to), this helps us to trace the status or previous comments of the patch. There are multiple change requests and comments to previous version, same seems sent in this version, can you please addresss them first? http://inbox.dpdk.org/dev/67d7272a-29fc-885f-ad8e-e865687e1267@intel.com/ > --- > doc/guides/nics/features/axgbe.ini | 1 + > drivers/net/axgbe/axgbe_ethdev.c | 2 + > drivers/net/axgbe/axgbe_phy.h | 4 ++ > drivers/net/axgbe/axgbe_phy_impl.c | 107 +++++++++++++++++++++++++++++ > 4 files changed, 114 insertions(+) > > diff --git a/doc/guides/nics/features/axgbe.ini b/doc/guides/nics/features/axgbe.ini > index 0becaa097..80a3bcee1 100644 > --- a/doc/guides/nics/features/axgbe.ini > +++ b/doc/guides/nics/features/axgbe.ini > @@ -18,3 +18,4 @@ Basic stats = Y > Linux UIO = Y > x86-32 = Y > x86-64 = Y > +Module EEPROM dump = Y Can you please add the items with same order as 'default.ini'
From: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> Adding API for get_module_eeprom and get_module_info.wq Signed-off-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> --- doc/guides/nics/features/axgbe.ini | 1 + drivers/net/axgbe/axgbe_ethdev.c | 2 + drivers/net/axgbe/axgbe_phy.h | 4 ++ drivers/net/axgbe/axgbe_phy_impl.c | 108 +++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+) diff --git a/doc/guides/nics/features/axgbe.ini b/doc/guides/nics/features/axgbe.ini index 34df0d1ee..3a090183e 100644 --- a/doc/guides/nics/features/axgbe.ini +++ b/doc/guides/nics/features/axgbe.ini @@ -17,6 +17,7 @@ CRC offload = Y L3 checksum offload = Y L4 checksum offload = Y Basic stats = Y +Module EEPROM dump = Y Linux UIO = Y x86-32 = Y x86-64 = Y diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c index cfe6aba73..22e05e8cb 100644 --- a/drivers/net/axgbe/axgbe_ethdev.c +++ b/drivers/net/axgbe/axgbe_ethdev.c @@ -257,6 +257,8 @@ static const struct eth_dev_ops axgbe_eth_dev_ops = { .timesync_adjust_time = axgbe_timesync_adjust_time, .timesync_read_time = axgbe_timesync_read_time, .timesync_write_time = axgbe_timesync_write_time, + .get_module_info = axgbe_get_module_info, + .get_module_eeprom = axgbe_get_module_eeprom, }; static int axgbe_phy_reset(struct axgbe_port *pdata) diff --git a/drivers/net/axgbe/axgbe_phy.h b/drivers/net/axgbe/axgbe_phy.h index 77ee20a31..d9d7fde41 100644 --- a/drivers/net/axgbe/axgbe_phy.h +++ b/drivers/net/axgbe/axgbe_phy.h @@ -188,5 +188,9 @@ #define DUPLEX_FULL 0x01 #define DUPLEX_UNKNOWN 0xff +int axgbe_get_module_info(struct rte_eth_dev *dev, + struct rte_eth_dev_module_info *modinfo); +int axgbe_get_module_eeprom(struct rte_eth_dev *dev, + struct rte_dev_eeprom_info *info); #endif /* PHY */ diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c index 02236ec19..af18a8d51 100644 --- a/drivers/net/axgbe/axgbe_phy_impl.c +++ b/drivers/net/axgbe/axgbe_phy_impl.c @@ -141,12 +141,18 @@ enum axgbe_sfp_speed { #define AXGBE_SFP_EXTD_CC 31 +#define AXGBE_SFP_EEPROM_PAGE_SIZE 256 + struct axgbe_sfp_eeprom { u8 base[64]; u8 extd[32]; u8 vendor[32]; }; +struct axgbe_sfp_eeprom_module { + u8 base[AXGBE_SFP_EEPROM_PAGE_SIZE]; +}; + #define AXGBE_BEL_FUSE_VENDOR "BEL-FUSE" #define AXGBE_BEL_FUSE_PARTNO "1GBT-SFP06" @@ -734,6 +740,108 @@ static int axgbe_phy_sfp_read_eeprom(struct axgbe_port *pdata) return ret; } +int axgbe_get_module_info(struct rte_eth_dev *dev, + struct rte_eth_dev_module_info *modinfo) +{ + struct axgbe_port *pdata = dev->data->dev_private; + struct axgbe_sfp_eeprom sfp_eeprom; + uint8_t eeprom_addr; + int ret; + + ret = axgbe_phy_get_comm_ownership(pdata); + + if (ret) + return -EIO; + + ret = axgbe_phy_sfp_get_mux(pdata); + + if (ret) { + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); + goto put; + } + + eeprom_addr = 0; + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, + &eeprom_addr, sizeof(eeprom_addr), + &sfp_eeprom, sizeof(sfp_eeprom)); + + if (ret) { + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); + goto put; + } + + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] != 0xff) { + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] == 0) { + modinfo->type = RTE_ETH_MODULE_SFF_8079; + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8079_LEN; + } else { + modinfo->type = RTE_ETH_MODULE_SFF_8472; + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8472_LEN; + } + } + +put: + axgbe_phy_sfp_put_mux(pdata); + axgbe_phy_put_comm_ownership(pdata); + return ret; +} + +int axgbe_get_module_eeprom(struct rte_eth_dev *dev, + struct rte_dev_eeprom_info *info) +{ + struct axgbe_port *pdata = dev->data->dev_private; + struct axgbe_sfp_eeprom_module sfp_eeprom; + uint8_t eeprom_addr; + uint8_t *data; + uint32_t i; + int ret; + + ret = axgbe_phy_get_comm_ownership(pdata); + + if (ret) + return -EIO; + + if (!info || !info->length || !info->data) { + axgbe_phy_put_comm_ownership(pdata); + return -EINVAL; + } + + ret = axgbe_phy_sfp_get_mux(pdata); + if (ret) { + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); + goto put; + } + + eeprom_addr = 0; + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, + &eeprom_addr, sizeof(eeprom_addr), + &sfp_eeprom, sizeof(sfp_eeprom)); + if (ret) { + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); + goto put; + } + data = info->data; + + /* for AXGBE_SFP_SERIAL_ID_ADDRESS */ + for (i = 0; i < AXGBE_SFP_EEPROM_PAGE_SIZE; i++) + data[i] = sfp_eeprom.base[i]; + + eeprom_addr = 0; + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_DIAG_INFO_ADDRESS, + &eeprom_addr, sizeof(eeprom_addr), + &sfp_eeprom, sizeof(sfp_eeprom)); + + /* for AXGBE_SFP_DIAG_INFO_ADDRESS */ + for (i = 0; i < info->length - AXGBE_SFP_EEPROM_PAGE_SIZE; i++) + data[i + AXGBE_SFP_EEPROM_PAGE_SIZE] = sfp_eeprom.base[i]; + +put: + axgbe_phy_sfp_put_mux(pdata); + axgbe_phy_put_comm_ownership(pdata); + return ret; +} + + static void axgbe_phy_sfp_signals(struct axgbe_port *pdata) { struct axgbe_phy_data *phy_data = pdata->phy_data; -- 2.25.1
On 12/22/2020 7:01 AM, asomalap@amd.com wrote: > From: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> > > Adding API for get_module_eeprom and get_module_info.wq > > Signed-off-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> > --- > doc/guides/nics/features/axgbe.ini | 1 + > drivers/net/axgbe/axgbe_ethdev.c | 2 + > drivers/net/axgbe/axgbe_phy.h | 4 ++ > drivers/net/axgbe/axgbe_phy_impl.c | 108 +++++++++++++++++++++++++++++ > 4 files changed, 115 insertions(+) > > diff --git a/doc/guides/nics/features/axgbe.ini b/doc/guides/nics/features/axgbe.ini > index 34df0d1ee..3a090183e 100644 > --- a/doc/guides/nics/features/axgbe.ini > +++ b/doc/guides/nics/features/axgbe.ini > @@ -17,6 +17,7 @@ CRC offload = Y > L3 checksum offload = Y > L4 checksum offload = Y > Basic stats = Y > +Module EEPROM dump = Y > Linux UIO = Y > x86-32 = Y > x86-64 = Y > diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c > index cfe6aba73..22e05e8cb 100644 > --- a/drivers/net/axgbe/axgbe_ethdev.c > +++ b/drivers/net/axgbe/axgbe_ethdev.c > @@ -257,6 +257,8 @@ static const struct eth_dev_ops axgbe_eth_dev_ops = { > .timesync_adjust_time = axgbe_timesync_adjust_time, > .timesync_read_time = axgbe_timesync_read_time, > .timesync_write_time = axgbe_timesync_write_time, > + .get_module_info = axgbe_get_module_info, > + .get_module_eeprom = axgbe_get_module_eeprom, > }; > > static int axgbe_phy_reset(struct axgbe_port *pdata) > diff --git a/drivers/net/axgbe/axgbe_phy.h b/drivers/net/axgbe/axgbe_phy.h > index 77ee20a31..d9d7fde41 100644 > --- a/drivers/net/axgbe/axgbe_phy.h > +++ b/drivers/net/axgbe/axgbe_phy.h > @@ -188,5 +188,9 @@ > #define DUPLEX_FULL 0x01 > #define DUPLEX_UNKNOWN 0xff > > +int axgbe_get_module_info(struct rte_eth_dev *dev, > + struct rte_eth_dev_module_info *modinfo); > +int axgbe_get_module_eeprom(struct rte_eth_dev *dev, > + struct rte_dev_eeprom_info *info); > #endif > /* PHY */ > diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c > index 02236ec19..af18a8d51 100644 > --- a/drivers/net/axgbe/axgbe_phy_impl.c > +++ b/drivers/net/axgbe/axgbe_phy_impl.c > @@ -141,12 +141,18 @@ enum axgbe_sfp_speed { > > #define AXGBE_SFP_EXTD_CC 31 > > +#define AXGBE_SFP_EEPROM_PAGE_SIZE 256 > + > struct axgbe_sfp_eeprom { > u8 base[64]; > u8 extd[32]; > u8 vendor[32]; > }; > > +struct axgbe_sfp_eeprom_module { > + u8 base[AXGBE_SFP_EEPROM_PAGE_SIZE]; > +}; > + > #define AXGBE_BEL_FUSE_VENDOR "BEL-FUSE" > #define AXGBE_BEL_FUSE_PARTNO "1GBT-SFP06" > > @@ -734,6 +740,108 @@ static int axgbe_phy_sfp_read_eeprom(struct axgbe_port *pdata) > return ret; > } > > +int axgbe_get_module_info(struct rte_eth_dev *dev, > + struct rte_eth_dev_module_info *modinfo) > +{ > + struct axgbe_port *pdata = dev->data->dev_private; > + struct axgbe_sfp_eeprom sfp_eeprom; > + uint8_t eeprom_addr; > + int ret; > + > + ret = axgbe_phy_get_comm_ownership(pdata); > + > + if (ret) > + return -EIO; > + > + ret = axgbe_phy_sfp_get_mux(pdata); > + > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); > + goto put; > + } > + > + eeprom_addr = 0; > + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, > + &eeprom_addr, sizeof(eeprom_addr), > + &sfp_eeprom, sizeof(sfp_eeprom)); > + > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); > + goto put; > + } > + > + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] != 0xff) { The 'else' case of this check seems wrong, which returns success but doesn't set modinfo correctly. > + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] == 0) { > + modinfo->type = RTE_ETH_MODULE_SFF_8079; > + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8079_LEN; > + } else { > + modinfo->type = RTE_ETH_MODULE_SFF_8472; > + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8472_LEN; > + } > + } > + > +put: > + axgbe_phy_sfp_put_mux(pdata); > + axgbe_phy_put_comm_ownership(pdata); > + return ret; > +} > + > +int axgbe_get_module_eeprom(struct rte_eth_dev *dev, > + struct rte_dev_eeprom_info *info) > +{ > + struct axgbe_port *pdata = dev->data->dev_private; > + struct axgbe_sfp_eeprom_module sfp_eeprom; > + uint8_t eeprom_addr; > + uint8_t *data; > + uint32_t i; > + int ret; > + > + ret = axgbe_phy_get_comm_ownership(pdata); > + > + if (ret) > + return -EIO; > + > + if (!info || !info->length || !info->data) { Up to you but it may be easier to verify the input and fail before taking the ownership of the device first. > + axgbe_phy_put_comm_ownership(pdata); > + return -EINVAL; > + } > + > + ret = axgbe_phy_sfp_get_mux(pdata); > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); > + goto put; > + } Just a syntax issue, some lines has an empty line between 'ret' assignment and 'ret' check, but some does not (as above), can you please make it consistent. > + > + eeprom_addr = 0; > + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, > + &eeprom_addr, sizeof(eeprom_addr), > + &sfp_eeprom, sizeof(sfp_eeprom)); > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); > + goto put; > + } > + data = info->data; > + > + /* for AXGBE_SFP_SERIAL_ID_ADDRESS */ > + for (i = 0; i < AXGBE_SFP_EEPROM_PAGE_SIZE; i++) > + data[i] = sfp_eeprom.base[i]; Should check if 'info->length' > 'AXGBE_SFP_SERIAL_ID_ADDRESS' before above assignment. > + > + eeprom_addr = 0; > + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_DIAG_INFO_ADDRESS, > + &eeprom_addr, sizeof(eeprom_addr), > + &sfp_eeprom, sizeof(sfp_eeprom)); > + > + /* for AXGBE_SFP_DIAG_INFO_ADDRESS */ > + for (i = 0; i < info->length - AXGBE_SFP_EEPROM_PAGE_SIZE; i++) > + data[i + AXGBE_SFP_EEPROM_PAGE_SIZE] = sfp_eeprom.base[i]; > + Will it be acceptable if the loop break in the middle because of missing space, "i < info->length - AXGBE_SFP_EEPROM_PAGE_SIZE"? If not, size needs to be checked before the loop. > +put: > + axgbe_phy_sfp_put_mux(pdata); > + axgbe_phy_put_comm_ownership(pdata); > + return ret; > +} > + > + > static void axgbe_phy_sfp_signals(struct axgbe_port *pdata) > { > struct axgbe_phy_data *phy_data = pdata->phy_data; >
-----Original Message----- From: Ferruh Yigit <ferruh.yigit@intel.com> Sent: Monday, January 4, 2021 7:54 PM To: Somalapuram, Amaranath <Amaranath.Somalapuram@amd.com>; dev@dpdk.org Subject: Re: [PATCH v4] net/axgbe: support sfp module EEPROM [CAUTION: External Email] On 12/22/2020 7:01 AM, asomalap@amd.com wrote: > From: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> > > Adding API for get_module_eeprom and get_module_info.wq > > Signed-off-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> > --- > doc/guides/nics/features/axgbe.ini | 1 + > drivers/net/axgbe/axgbe_ethdev.c | 2 + > drivers/net/axgbe/axgbe_phy.h | 4 ++ > drivers/net/axgbe/axgbe_phy_impl.c | 108 +++++++++++++++++++++++++++++ > 4 files changed, 115 insertions(+) > > diff --git a/doc/guides/nics/features/axgbe.ini > b/doc/guides/nics/features/axgbe.ini > index 34df0d1ee..3a090183e 100644 > --- a/doc/guides/nics/features/axgbe.ini > +++ b/doc/guides/nics/features/axgbe.ini > @@ -17,6 +17,7 @@ CRC offload = Y > L3 checksum offload = Y > L4 checksum offload = Y > Basic stats = Y > +Module EEPROM dump = Y > Linux UIO = Y > x86-32 = Y > x86-64 = Y > diff --git a/drivers/net/axgbe/axgbe_ethdev.c > b/drivers/net/axgbe/axgbe_ethdev.c > index cfe6aba73..22e05e8cb 100644 > --- a/drivers/net/axgbe/axgbe_ethdev.c > +++ b/drivers/net/axgbe/axgbe_ethdev.c > @@ -257,6 +257,8 @@ static const struct eth_dev_ops axgbe_eth_dev_ops = { > .timesync_adjust_time = axgbe_timesync_adjust_time, > .timesync_read_time = axgbe_timesync_read_time, > .timesync_write_time = axgbe_timesync_write_time, > + .get_module_info = axgbe_get_module_info, > + .get_module_eeprom = axgbe_get_module_eeprom, > }; > > static int axgbe_phy_reset(struct axgbe_port *pdata) diff --git > a/drivers/net/axgbe/axgbe_phy.h b/drivers/net/axgbe/axgbe_phy.h index > 77ee20a31..d9d7fde41 100644 > --- a/drivers/net/axgbe/axgbe_phy.h > +++ b/drivers/net/axgbe/axgbe_phy.h > @@ -188,5 +188,9 @@ > #define DUPLEX_FULL 0x01 > #define DUPLEX_UNKNOWN 0xff > > +int axgbe_get_module_info(struct rte_eth_dev *dev, > + struct rte_eth_dev_module_info > +*modinfo); int axgbe_get_module_eeprom(struct rte_eth_dev *dev, > + struct rte_dev_eeprom_info *info); > #endif > /* PHY */ > diff --git a/drivers/net/axgbe/axgbe_phy_impl.c > b/drivers/net/axgbe/axgbe_phy_impl.c > index 02236ec19..af18a8d51 100644 > --- a/drivers/net/axgbe/axgbe_phy_impl.c > +++ b/drivers/net/axgbe/axgbe_phy_impl.c > @@ -141,12 +141,18 @@ enum axgbe_sfp_speed { > > #define AXGBE_SFP_EXTD_CC 31 > > +#define AXGBE_SFP_EEPROM_PAGE_SIZE 256 > + > struct axgbe_sfp_eeprom { > u8 base[64]; > u8 extd[32]; > u8 vendor[32]; > }; > > +struct axgbe_sfp_eeprom_module { > + u8 base[AXGBE_SFP_EEPROM_PAGE_SIZE]; }; > + > #define AXGBE_BEL_FUSE_VENDOR "BEL-FUSE" > #define AXGBE_BEL_FUSE_PARTNO "1GBT-SFP06" > > @@ -734,6 +740,108 @@ static int axgbe_phy_sfp_read_eeprom(struct axgbe_port *pdata) > return ret; > } > > +int axgbe_get_module_info(struct rte_eth_dev *dev, > + struct rte_eth_dev_module_info *modinfo) > +{ > + struct axgbe_port *pdata = dev->data->dev_private; > + struct axgbe_sfp_eeprom sfp_eeprom; > + uint8_t eeprom_addr; > + int ret; > + > + ret = axgbe_phy_get_comm_ownership(pdata); > + > + if (ret) > + return -EIO; > + > + ret = axgbe_phy_sfp_get_mux(pdata); > + > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); > + goto put; > + } > + > + eeprom_addr = 0; > + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, > + &eeprom_addr, sizeof(eeprom_addr), > + &sfp_eeprom, sizeof(sfp_eeprom)); > + > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); > + goto put; > + } > + > + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] != 0xff) { The 'else' case of this check seems wrong, which returns success but doesn't set modinfo correctly. I have used the reference code from i40e_ethdev.c and ixgbe_ethdev.c. let me know your inputs. > + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] == 0) { > + modinfo->type = RTE_ETH_MODULE_SFF_8079; > + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8079_LEN; > + } else { > + modinfo->type = RTE_ETH_MODULE_SFF_8472; > + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8472_LEN; > + } > + } > + > +put: > + axgbe_phy_sfp_put_mux(pdata); > + axgbe_phy_put_comm_ownership(pdata); > + return ret; > +} > + > +int axgbe_get_module_eeprom(struct rte_eth_dev *dev, > + struct rte_dev_eeprom_info *info) { > + struct axgbe_port *pdata = dev->data->dev_private; > + struct axgbe_sfp_eeprom_module sfp_eeprom; > + uint8_t eeprom_addr; > + uint8_t *data; > + uint32_t i; > + int ret; > + > + ret = axgbe_phy_get_comm_ownership(pdata); > + > + if (ret) > + return -EIO; > + > + if (!info || !info->length || !info->data) { Up to you but it may be easier to verify the input and fail before taking the ownership of the device first. Agreed. > + axgbe_phy_put_comm_ownership(pdata); > + return -EINVAL; > + } > + > + ret = axgbe_phy_sfp_get_mux(pdata); > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); > + goto put; > + } Just a syntax issue, some lines has an empty line between 'ret' assignment and 'ret' check, but some does not (as above), can you please make it consistent. Agreed. > + > + eeprom_addr = 0; > + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, > + &eeprom_addr, sizeof(eeprom_addr), > + &sfp_eeprom, sizeof(sfp_eeprom)); > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); > + goto put; > + } > + data = info->data; > + > + /* for AXGBE_SFP_SERIAL_ID_ADDRESS */ > + for (i = 0; i < AXGBE_SFP_EEPROM_PAGE_SIZE; i++) > + data[i] = sfp_eeprom.base[i]; Should check if 'info->length' > 'AXGBE_SFP_SERIAL_ID_ADDRESS' before above assignment. Agreed. > + > + eeprom_addr = 0; > + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_DIAG_INFO_ADDRESS, > + &eeprom_addr, sizeof(eeprom_addr), > + &sfp_eeprom, sizeof(sfp_eeprom)); > + > + /* for AXGBE_SFP_DIAG_INFO_ADDRESS */ > + for (i = 0; i < info->length - AXGBE_SFP_EEPROM_PAGE_SIZE; i++) > + data[i + AXGBE_SFP_EEPROM_PAGE_SIZE] = > + sfp_eeprom.base[i]; > + Will it be acceptable if the loop break in the middle because of missing space, "i < info->length - AXGBE_SFP_EEPROM_PAGE_SIZE"? If not, size needs to be checked before the loop. Agreed. > +put: > + axgbe_phy_sfp_put_mux(pdata); > + axgbe_phy_put_comm_ownership(pdata); > + return ret; > +} > + > + > static void axgbe_phy_sfp_signals(struct axgbe_port *pdata) > { > struct axgbe_phy_data *phy_data = pdata->phy_data; > Please reply for the else case issue. I can resubmit. Other I will fix.
On 1/6/2021 12:31 PM, Somalapuram, Amaranath wrote: > > > -----Original Message----- > From: Ferruh Yigit <ferruh.yigit@intel.com> > Sent: Monday, January 4, 2021 7:54 PM > To: Somalapuram, Amaranath <Amaranath.Somalapuram@amd.com>; dev@dpdk.org > Subject: Re: [PATCH v4] net/axgbe: support sfp module EEPROM > > [CAUTION: External Email] > > On 12/22/2020 7:01 AM, asomalap@amd.com wrote: >> From: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> >> >> Adding API for get_module_eeprom and get_module_info.wq >> >> Signed-off-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> >> --- >> doc/guides/nics/features/axgbe.ini | 1 + >> drivers/net/axgbe/axgbe_ethdev.c | 2 + >> drivers/net/axgbe/axgbe_phy.h | 4 ++ >> drivers/net/axgbe/axgbe_phy_impl.c | 108 +++++++++++++++++++++++++++++ >> 4 files changed, 115 insertions(+) >> >> diff --git a/doc/guides/nics/features/axgbe.ini >> b/doc/guides/nics/features/axgbe.ini >> index 34df0d1ee..3a090183e 100644 >> --- a/doc/guides/nics/features/axgbe.ini >> +++ b/doc/guides/nics/features/axgbe.ini >> @@ -17,6 +17,7 @@ CRC offload = Y >> L3 checksum offload = Y >> L4 checksum offload = Y >> Basic stats = Y >> +Module EEPROM dump = Y >> Linux UIO = Y >> x86-32 = Y >> x86-64 = Y >> diff --git a/drivers/net/axgbe/axgbe_ethdev.c >> b/drivers/net/axgbe/axgbe_ethdev.c >> index cfe6aba73..22e05e8cb 100644 >> --- a/drivers/net/axgbe/axgbe_ethdev.c >> +++ b/drivers/net/axgbe/axgbe_ethdev.c >> @@ -257,6 +257,8 @@ static const struct eth_dev_ops axgbe_eth_dev_ops = { >> .timesync_adjust_time = axgbe_timesync_adjust_time, >> .timesync_read_time = axgbe_timesync_read_time, >> .timesync_write_time = axgbe_timesync_write_time, >> + .get_module_info = axgbe_get_module_info, >> + .get_module_eeprom = axgbe_get_module_eeprom, >> }; >> >> static int axgbe_phy_reset(struct axgbe_port *pdata) diff --git >> a/drivers/net/axgbe/axgbe_phy.h b/drivers/net/axgbe/axgbe_phy.h index >> 77ee20a31..d9d7fde41 100644 >> --- a/drivers/net/axgbe/axgbe_phy.h >> +++ b/drivers/net/axgbe/axgbe_phy.h >> @@ -188,5 +188,9 @@ >> #define DUPLEX_FULL 0x01 >> #define DUPLEX_UNKNOWN 0xff >> >> +int axgbe_get_module_info(struct rte_eth_dev *dev, >> + struct rte_eth_dev_module_info >> +*modinfo); int axgbe_get_module_eeprom(struct rte_eth_dev *dev, >> + struct rte_dev_eeprom_info *info); >> #endif >> /* PHY */ >> diff --git a/drivers/net/axgbe/axgbe_phy_impl.c >> b/drivers/net/axgbe/axgbe_phy_impl.c >> index 02236ec19..af18a8d51 100644 >> --- a/drivers/net/axgbe/axgbe_phy_impl.c >> +++ b/drivers/net/axgbe/axgbe_phy_impl.c >> @@ -141,12 +141,18 @@ enum axgbe_sfp_speed { >> >> #define AXGBE_SFP_EXTD_CC 31 >> >> +#define AXGBE_SFP_EEPROM_PAGE_SIZE 256 >> + >> struct axgbe_sfp_eeprom { >> u8 base[64]; >> u8 extd[32]; >> u8 vendor[32]; >> }; >> >> +struct axgbe_sfp_eeprom_module { >> + u8 base[AXGBE_SFP_EEPROM_PAGE_SIZE]; }; >> + >> #define AXGBE_BEL_FUSE_VENDOR "BEL-FUSE" >> #define AXGBE_BEL_FUSE_PARTNO "1GBT-SFP06" >> >> @@ -734,6 +740,108 @@ static int axgbe_phy_sfp_read_eeprom(struct axgbe_port *pdata) >> return ret; >> } >> >> +int axgbe_get_module_info(struct rte_eth_dev *dev, >> + struct rte_eth_dev_module_info *modinfo) >> +{ >> + struct axgbe_port *pdata = dev->data->dev_private; >> + struct axgbe_sfp_eeprom sfp_eeprom; >> + uint8_t eeprom_addr; >> + int ret; >> + >> + ret = axgbe_phy_get_comm_ownership(pdata); >> + >> + if (ret) >> + return -EIO; >> + >> + ret = axgbe_phy_sfp_get_mux(pdata); >> + >> + if (ret) { >> + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); >> + goto put; >> + } >> + >> + eeprom_addr = 0; >> + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, >> + &eeprom_addr, sizeof(eeprom_addr), >> + &sfp_eeprom, sizeof(sfp_eeprom)); >> + >> + if (ret) { >> + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); >> + goto put; >> + } >> + >> + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] != 0xff) { > > The 'else' case of this check seems wrong, which returns success but doesn't set modinfo correctly. > > I have used the reference code from i40e_ethdev.c and ixgbe_ethdev.c. let me know your inputs. > I am not sure what exactly you are referencing in i40e & ixgbe, as far as I can see there is no similar usage on them, if there is please show it to me. Above issue is, what if "sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] == 0xff", what will happen in that case? As far as I can see in that case function returns success, but the 'modinfo' is not updated at all, am I missing something? >> + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] == 0) { >> + modinfo->type = RTE_ETH_MODULE_SFF_8079; >> + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8079_LEN; >> + } else { >> + modinfo->type = RTE_ETH_MODULE_SFF_8472; >> + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8472_LEN; >> + } >> + } >> + >> +put: >> + axgbe_phy_sfp_put_mux(pdata); >> + axgbe_phy_put_comm_ownership(pdata); >> + return ret; >> +} >> + <...>
From: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> Adding API for get_module_eeprom and get_module_info. Signed-off-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> --- doc/guides/nics/features/axgbe.ini | 1 + drivers/net/axgbe/axgbe_ethdev.c | 2 + drivers/net/axgbe/axgbe_phy.h | 4 + drivers/net/axgbe/axgbe_phy_impl.c | 114 +++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+) diff --git a/doc/guides/nics/features/axgbe.ini b/doc/guides/nics/features/axgbe.ini index 34df0d1ee..3a090183e 100644 --- a/doc/guides/nics/features/axgbe.ini +++ b/doc/guides/nics/features/axgbe.ini @@ -17,6 +17,7 @@ CRC offload = Y L3 checksum offload = Y L4 checksum offload = Y Basic stats = Y +Module EEPROM dump = Y Linux UIO = Y x86-32 = Y x86-64 = Y diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c index cfe6aba73..22e05e8cb 100644 --- a/drivers/net/axgbe/axgbe_ethdev.c +++ b/drivers/net/axgbe/axgbe_ethdev.c @@ -257,6 +257,8 @@ static const struct eth_dev_ops axgbe_eth_dev_ops = { .timesync_adjust_time = axgbe_timesync_adjust_time, .timesync_read_time = axgbe_timesync_read_time, .timesync_write_time = axgbe_timesync_write_time, + .get_module_info = axgbe_get_module_info, + .get_module_eeprom = axgbe_get_module_eeprom, }; static int axgbe_phy_reset(struct axgbe_port *pdata) diff --git a/drivers/net/axgbe/axgbe_phy.h b/drivers/net/axgbe/axgbe_phy.h index 77ee20a31..d9d7fde41 100644 --- a/drivers/net/axgbe/axgbe_phy.h +++ b/drivers/net/axgbe/axgbe_phy.h @@ -188,5 +188,9 @@ #define DUPLEX_FULL 0x01 #define DUPLEX_UNKNOWN 0xff +int axgbe_get_module_info(struct rte_eth_dev *dev, + struct rte_eth_dev_module_info *modinfo); +int axgbe_get_module_eeprom(struct rte_eth_dev *dev, + struct rte_dev_eeprom_info *info); #endif /* PHY */ diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c index 02236ec19..e60f51ede 100644 --- a/drivers/net/axgbe/axgbe_phy_impl.c +++ b/drivers/net/axgbe/axgbe_phy_impl.c @@ -141,12 +141,18 @@ enum axgbe_sfp_speed { #define AXGBE_SFP_EXTD_CC 31 +#define AXGBE_SFP_EEPROM_PAGE_SIZE 256 + struct axgbe_sfp_eeprom { u8 base[64]; u8 extd[32]; u8 vendor[32]; }; +struct axgbe_sfp_eeprom_module { + u8 base[AXGBE_SFP_EEPROM_PAGE_SIZE]; +}; + #define AXGBE_BEL_FUSE_VENDOR "BEL-FUSE" #define AXGBE_BEL_FUSE_PARTNO "1GBT-SFP06" @@ -734,6 +740,114 @@ static int axgbe_phy_sfp_read_eeprom(struct axgbe_port *pdata) return ret; } +int axgbe_get_module_info(struct rte_eth_dev *dev, + struct rte_eth_dev_module_info *modinfo) +{ + struct axgbe_port *pdata = dev->data->dev_private; + struct axgbe_sfp_eeprom sfp_eeprom; + uint8_t eeprom_addr; + int ret; + + ret = axgbe_phy_get_comm_ownership(pdata); + + if (ret) + return -EIO; + + ret = axgbe_phy_sfp_get_mux(pdata); + + if (ret) { + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); + goto put; + } + + eeprom_addr = 0; + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, + &eeprom_addr, sizeof(eeprom_addr), + &sfp_eeprom, sizeof(sfp_eeprom)); + + if (ret) { + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); + goto put; + } + + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] != 0xff) { + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] == 0) { + modinfo->type = RTE_ETH_MODULE_SFF_8079; + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8079_LEN; + } else { + ret = -EIO; + goto put; + } + } + +put: + axgbe_phy_sfp_put_mux(pdata); + axgbe_phy_put_comm_ownership(pdata); + return ret; +} + +int axgbe_get_module_eeprom(struct rte_eth_dev *dev, + struct rte_dev_eeprom_info *info) +{ + struct axgbe_port *pdata = dev->data->dev_private; + struct axgbe_sfp_eeprom_module sfp_eeprom; + uint8_t eeprom_addr; + uint8_t *data; + uint32_t i; + int ret; + + if (!info || !info->length || !info->data) { + axgbe_phy_put_comm_ownership(pdata); + return -EINVAL; + } + + ret = axgbe_phy_get_comm_ownership(pdata); + + if (ret) + return -EIO; + + ret = axgbe_phy_sfp_get_mux(pdata); + + if (ret) { + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); + goto put; + } + + eeprom_addr = 0; + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, + &eeprom_addr, sizeof(eeprom_addr), + &sfp_eeprom, sizeof(sfp_eeprom)); + + if (ret) { + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); + goto put; + } + + data = info->data; + + /* for AXGBE_SFP_SERIAL_ID_ADDRESS */ + if (info->length > AXGBE_SFP_EEPROM_PAGE_SIZE) + for (i = 0; i < AXGBE_SFP_EEPROM_PAGE_SIZE; i++) + data[i] = sfp_eeprom.base[i]; + + eeprom_addr = 0; + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_DIAG_INFO_ADDRESS, + &eeprom_addr, sizeof(eeprom_addr), + &sfp_eeprom, sizeof(sfp_eeprom)); + + /* for AXGBE_SFP_DIAG_INFO_ADDRESS */ + if (info->length > AXGBE_SFP_EEPROM_PAGE_SIZE) + for (i = 0; i < info->length - AXGBE_SFP_EEPROM_PAGE_SIZE; i++) + data[i + AXGBE_SFP_EEPROM_PAGE_SIZE] = + sfp_eeprom.base[i]; + +put: + axgbe_phy_sfp_put_mux(pdata); + axgbe_phy_put_comm_ownership(pdata); + return ret; +} + + static void axgbe_phy_sfp_signals(struct axgbe_port *pdata) { struct axgbe_phy_data *phy_data = pdata->phy_data; -- 2.25.1
On 1/12/2021 5:28 AM, asomalap@amd.com wrote: > From: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> > > Adding API for get_module_eeprom and get_module_info. > > Signed-off-by: Amaranath Somalapuram <Amaranath.Somalapuram@amd.com> > --- > doc/guides/nics/features/axgbe.ini | 1 + > drivers/net/axgbe/axgbe_ethdev.c | 2 + > drivers/net/axgbe/axgbe_phy.h | 4 + > drivers/net/axgbe/axgbe_phy_impl.c | 114 +++++++++++++++++++++++++++++ > 4 files changed, 121 insertions(+) > > diff --git a/doc/guides/nics/features/axgbe.ini b/doc/guides/nics/features/axgbe.ini > index 34df0d1ee..3a090183e 100644 > --- a/doc/guides/nics/features/axgbe.ini > +++ b/doc/guides/nics/features/axgbe.ini > @@ -17,6 +17,7 @@ CRC offload = Y > L3 checksum offload = Y > L4 checksum offload = Y > Basic stats = Y > +Module EEPROM dump = Y > Linux UIO = Y > x86-32 = Y > x86-64 = Y > diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c > index cfe6aba73..22e05e8cb 100644 > --- a/drivers/net/axgbe/axgbe_ethdev.c > +++ b/drivers/net/axgbe/axgbe_ethdev.c > @@ -257,6 +257,8 @@ static const struct eth_dev_ops axgbe_eth_dev_ops = { > .timesync_adjust_time = axgbe_timesync_adjust_time, > .timesync_read_time = axgbe_timesync_read_time, > .timesync_write_time = axgbe_timesync_write_time, > + .get_module_info = axgbe_get_module_info, > + .get_module_eeprom = axgbe_get_module_eeprom, > }; > > static int axgbe_phy_reset(struct axgbe_port *pdata) > diff --git a/drivers/net/axgbe/axgbe_phy.h b/drivers/net/axgbe/axgbe_phy.h > index 77ee20a31..d9d7fde41 100644 > --- a/drivers/net/axgbe/axgbe_phy.h > +++ b/drivers/net/axgbe/axgbe_phy.h > @@ -188,5 +188,9 @@ > #define DUPLEX_FULL 0x01 > #define DUPLEX_UNKNOWN 0xff > > +int axgbe_get_module_info(struct rte_eth_dev *dev, > + struct rte_eth_dev_module_info *modinfo); > +int axgbe_get_module_eeprom(struct rte_eth_dev *dev, > + struct rte_dev_eeprom_info *info); > #endif > /* PHY */ > diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c > index 02236ec19..e60f51ede 100644 > --- a/drivers/net/axgbe/axgbe_phy_impl.c > +++ b/drivers/net/axgbe/axgbe_phy_impl.c > @@ -141,12 +141,18 @@ enum axgbe_sfp_speed { > > #define AXGBE_SFP_EXTD_CC 31 > > +#define AXGBE_SFP_EEPROM_PAGE_SIZE 256 > + > struct axgbe_sfp_eeprom { > u8 base[64]; > u8 extd[32]; > u8 vendor[32]; > }; > > +struct axgbe_sfp_eeprom_module { > + u8 base[AXGBE_SFP_EEPROM_PAGE_SIZE]; > +}; > + > #define AXGBE_BEL_FUSE_VENDOR "BEL-FUSE" > #define AXGBE_BEL_FUSE_PARTNO "1GBT-SFP06" > > @@ -734,6 +740,114 @@ static int axgbe_phy_sfp_read_eeprom(struct axgbe_port *pdata) > return ret; > } > > +int axgbe_get_module_info(struct rte_eth_dev *dev, > + struct rte_eth_dev_module_info *modinfo) > +{ > + struct axgbe_port *pdata = dev->data->dev_private; > + struct axgbe_sfp_eeprom sfp_eeprom; > + uint8_t eeprom_addr; > + int ret; > + > + ret = axgbe_phy_get_comm_ownership(pdata); > + > + if (ret) > + return -EIO; > + > + ret = axgbe_phy_sfp_get_mux(pdata); > + > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); > + goto put; > + } > + > + eeprom_addr = 0; > + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, > + &eeprom_addr, sizeof(eeprom_addr), > + &sfp_eeprom, sizeof(sfp_eeprom)); > + > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); > + goto put; > + } > + > + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] != 0xff) { > + if (sfp_eeprom.extd[AXGBE_SFP_EXTD_SFF_8472] == 0) { > + modinfo->type = RTE_ETH_MODULE_SFF_8079; > + modinfo->eeprom_len = RTE_ETH_MODULE_SFF_8079_LEN; > + } else { > + ret = -EIO; > + goto put; > + } > + } The issue mentioned is still valid, can you please check comment in the previous version(s). > + > +put: > + axgbe_phy_sfp_put_mux(pdata); > + axgbe_phy_put_comm_ownership(pdata); > + return ret; > +} > + > +int axgbe_get_module_eeprom(struct rte_eth_dev *dev, > + struct rte_dev_eeprom_info *info) > +{ > + struct axgbe_port *pdata = dev->data->dev_private; > + struct axgbe_sfp_eeprom_module sfp_eeprom; > + uint8_t eeprom_addr; > + uint8_t *data; > + uint32_t i; > + int ret; > + > + if (!info || !info->length || !info->data) { > + axgbe_phy_put_comm_ownership(pdata); > + return -EINVAL; > + } > + > + ret = axgbe_phy_get_comm_ownership(pdata); > + > + if (ret) > + return -EIO; > + > + ret = axgbe_phy_sfp_get_mux(pdata); > + > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error setting SFP MUX\n"); > + goto put; > + } > + > + eeprom_addr = 0; > + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_SERIAL_ID_ADDRESS, > + &eeprom_addr, sizeof(eeprom_addr), > + &sfp_eeprom, sizeof(sfp_eeprom)); > + > + if (ret) { > + PMD_DRV_LOG(ERR, "I2C error reading SFP EEPROM\n"); > + goto put; > + } > + > + data = info->data; > + > + /* for AXGBE_SFP_SERIAL_ID_ADDRESS */ > + if (info->length > AXGBE_SFP_EEPROM_PAGE_SIZE) > + for (i = 0; i < AXGBE_SFP_EEPROM_PAGE_SIZE; i++) > + data[i] = sfp_eeprom.base[i]; > + > + eeprom_addr = 0; > + ret = axgbe_phy_i2c_read(pdata, AXGBE_SFP_DIAG_INFO_ADDRESS, > + &eeprom_addr, sizeof(eeprom_addr), > + &sfp_eeprom, sizeof(sfp_eeprom)); > + > + /* for AXGBE_SFP_DIAG_INFO_ADDRESS */ > + if (info->length > AXGBE_SFP_EEPROM_PAGE_SIZE) > + for (i = 0; i < info->length - AXGBE_SFP_EEPROM_PAGE_SIZE; i++) > + data[i + AXGBE_SFP_EEPROM_PAGE_SIZE] = > + sfp_eeprom.base[i]; The second page can be written partially, isn't this a problem? > + > +put: > + axgbe_phy_sfp_put_mux(pdata); > + axgbe_phy_put_comm_ownership(pdata); > + return ret; > +} > + > + > static void axgbe_phy_sfp_signals(struct axgbe_port *pdata) > { > struct axgbe_phy_data *phy_data = pdata->phy_data; >