DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1] net/axgbe: support sfp module EEPROM
@ 2020-05-14 13:43 asomalap
  2020-05-14 16:35 ` Ferruh Yigit
  0 siblings, 1 reply; 8+ messages in thread
From: asomalap @ 2020-05-14 13:43 UTC (permalink / raw)
  To: dev; +Cc: asomalap

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v1] net/axgbe: support sfp module EEPROM
  2020-05-14 13:43 [dpdk-dev] [PATCH v1] net/axgbe: support sfp module EEPROM asomalap
@ 2020-05-14 16:35 ` Ferruh Yigit
  2020-12-22  7:01   ` [dpdk-dev] [PATCH v4] " asomalap
  0 siblings, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2020-05-14 16:35 UTC (permalink / raw)
  To: asomalap, dev

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'

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH v4] net/axgbe: support sfp module EEPROM
  2020-05-14 16:35 ` Ferruh Yigit
@ 2020-12-22  7:01   ` asomalap
  2021-01-04 14:23     ` Ferruh Yigit
  2021-01-12  5:28     ` [dpdk-dev] [PATCH v5] " asomalap
  0 siblings, 2 replies; 8+ messages in thread
From: asomalap @ 2020-12-22  7:01 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v4] net/axgbe: support sfp module EEPROM
  2020-12-22  7:01   ` [dpdk-dev] [PATCH v4] " asomalap
@ 2021-01-04 14:23     ` Ferruh Yigit
  2021-01-06 12:31       ` Somalapuram, Amaranath
  2021-01-12  5:28     ` [dpdk-dev] [PATCH v5] " asomalap
  1 sibling, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2021-01-04 14:23 UTC (permalink / raw)
  To: asomalap, dev

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;
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v4] net/axgbe: support sfp module EEPROM
  2021-01-04 14:23     ` Ferruh Yigit
@ 2021-01-06 12:31       ` Somalapuram, Amaranath
  2021-01-06 14:02         ` Ferruh Yigit
  0 siblings, 1 reply; 8+ messages in thread
From: Somalapuram, Amaranath @ 2021-01-06 12:31 UTC (permalink / raw)
  To: Ferruh Yigit, dev



-----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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v4] net/axgbe: support sfp module EEPROM
  2021-01-06 12:31       ` Somalapuram, Amaranath
@ 2021-01-06 14:02         ` Ferruh Yigit
  0 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2021-01-06 14:02 UTC (permalink / raw)
  To: Somalapuram, Amaranath, dev

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;
>> +}
>> +

<...>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH v5] net/axgbe: support sfp module EEPROM
  2020-12-22  7:01   ` [dpdk-dev] [PATCH v4] " asomalap
  2021-01-04 14:23     ` Ferruh Yigit
@ 2021-01-12  5:28     ` asomalap
  2021-01-18 15:17       ` Ferruh Yigit
  1 sibling, 1 reply; 8+ messages in thread
From: asomalap @ 2021-01-12  5:28 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v5] net/axgbe: support sfp module EEPROM
  2021-01-12  5:28     ` [dpdk-dev] [PATCH v5] " asomalap
@ 2021-01-18 15:17       ` Ferruh Yigit
  0 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2021-01-18 15:17 UTC (permalink / raw)
  To: asomalap, dev

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;
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-01-18 15:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-14 13:43 [dpdk-dev] [PATCH v1] net/axgbe: support sfp module EEPROM asomalap
2020-05-14 16:35 ` Ferruh Yigit
2020-12-22  7:01   ` [dpdk-dev] [PATCH v4] " asomalap
2021-01-04 14:23     ` Ferruh Yigit
2021-01-06 12:31       ` Somalapuram, Amaranath
2021-01-06 14:02         ` Ferruh Yigit
2021-01-12  5:28     ` [dpdk-dev] [PATCH v5] " asomalap
2021-01-18 15:17       ` Ferruh Yigit

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git