[dpdk-dev] [PATCH v1] net/axgbe: add support for utility APIs

[dpdk-dev] [PATCH v1] net/axgbe: add support for utility APIs

[selwin.sebastian]

From: Selwin Sebastian <selwin.sebastian@amd.com>

Add support for rx, tx queue utility APIs
Add support for 'fw_revision_get' API

Signed-off-by: Selwin Sebastian <selwin.sebastian@amd.com>
---
 drivers/net/axgbe/axgbe_ethdev.c |  7 +++
 drivers/net/axgbe/axgbe_rxtx.c   | 89 ++++++++++++++++++++++++++++++++
 drivers/net/axgbe/axgbe_rxtx.h   |  4 ++
 3 files changed, 100 insertions(+)

diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index 867058845..e943d1dae 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -214,6 +214,11 @@ 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,
+	.rx_queue_start       = axgbe_dev_rx_queue_start,
+	.rx_queue_stop        = axgbe_dev_rx_queue_stop,
+	.tx_queue_start       = axgbe_dev_tx_queue_start,
+	.tx_queue_stop        = axgbe_dev_tx_queue_stop,
+	.fw_version_get       = axgbe_dev_fw_version_get,
 };
 
 static int axgbe_phy_reset(struct axgbe_port *pdata)
@@ -1006,6 +1011,8 @@ axgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_TX_OFFLOAD_IPV4_CKSUM  |
 		DEV_TX_OFFLOAD_UDP_CKSUM   |
 		DEV_TX_OFFLOAD_TCP_CKSUM;
+	dev_info->dev_capa = RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP |
+			RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP;
 
 	if (pdata->hw_feat.rss) {
 		dev_info->flow_type_rss_offloads = AXGBE_RSS_OFFLOAD;
diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index 30c467db7..495e07902 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -565,6 +565,95 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 	return 0;
 }
 
+int axgbe_dev_tx_queue_start(struct rte_eth_dev *eth_dev,
+		uint16_t queue_idx)
+{
+	struct axgbe_port *pdata =
+		(struct axgbe_port *)eth_dev->data->dev_private;
+	int ret = 0;
+
+	PMD_INIT_FUNC_TRACE();
+
+	AXGMAC_MTL_IOWRITE_BITS(pdata, queue_idx,
+			MTL_Q_TQOMR, TXQEN, MTL_Q_ENABLED);
+	eth_dev->data->tx_queue_state[queue_idx] = RTE_ETH_QUEUE_STATE_STARTED;
+
+	return ret;
+}
+
+int axgbe_dev_tx_queue_stop(struct rte_eth_dev *eth_dev,
+		uint16_t queue_idx)
+{
+	int ret = 0;
+	struct axgbe_port *pdata =
+		(struct axgbe_port *)eth_dev->data->dev_private;
+
+	PMD_INIT_FUNC_TRACE();
+
+	AXGMAC_MTL_IOWRITE_BITS(pdata, queue_idx, MTL_Q_TQOMR, TXQEN, 0);
+	eth_dev->data->tx_queue_state[queue_idx] = RTE_ETH_QUEUE_STATE_STOPPED;
+
+	return ret;
+}
+
+int axgbe_dev_rx_queue_start(struct rte_eth_dev *eth_dev,
+		uint16_t queue_idx)
+{
+	struct axgbe_port *pdata =
+		(struct axgbe_port *)eth_dev->data->dev_private;
+	unsigned int reg_val = 0;
+	int ret = 0;
+
+	PMD_INIT_FUNC_TRACE();
+
+	reg_val |= (0x02 << (queue_idx << 1));
+	AXGMAC_IOWRITE(pdata, MAC_RQC0R, reg_val);
+	eth_dev->data->rx_queue_state[queue_idx] = RTE_ETH_QUEUE_STATE_STARTED;
+
+	return ret;
+}
+
+int axgbe_dev_rx_queue_stop(struct rte_eth_dev *eth_dev,
+		uint16_t queue_idx)
+{
+	int ret = 0;
+	struct axgbe_port *pdata =
+		(struct axgbe_port *)eth_dev->data->dev_private;
+	struct axgbe_rx_queue *rxq;
+
+	PMD_INIT_FUNC_TRACE();
+
+	AXGMAC_IOWRITE(pdata, MAC_RQC0R, 0);
+	rxq = eth_dev->data->rx_queues[queue_idx];
+
+	/* Disable Rx DMA channel */
+	AXGMAC_DMA_IOWRITE_BITS(rxq, DMA_CH_RCR, SR, 0);
+	eth_dev->data->rx_queue_state[queue_idx] = RTE_ETH_QUEUE_STATE_STOPPED;
+
+	return ret;
+}
+
+int axgbe_dev_fw_version_get(struct rte_eth_dev *eth_dev,
+		char *fw_version, size_t fw_size)
+{
+	struct axgbe_port *pdata;
+	struct axgbe_hw_features *hw_feat;
+	char fw_ver[32];
+
+	pdata = (struct axgbe_port *)eth_dev->data->dev_private;
+	hw_feat = &pdata->hw_feat;
+
+	if (fw_version == NULL || fw_size <= 0)
+		return -EINVAL;
+
+	snprintf(fw_version, sizeof(fw_ver), "%d.%d.%d",
+			AXGMAC_GET_BITS(hw_feat->version, MAC_VR, USERVER),
+			AXGMAC_GET_BITS(hw_feat->version, MAC_VR, DEVID),
+			AXGMAC_GET_BITS(hw_feat->version, MAC_VR, SNPSVER));
+
+	return 0;
+}
+
 static void axgbe_txq_prepare_tx_stop(struct axgbe_port *pdata,
 				      unsigned int queue)
 {
diff --git a/drivers/net/axgbe/axgbe_rxtx.h b/drivers/net/axgbe/axgbe_rxtx.h
index f2fbe9299..3e05e0e6f 100644
--- a/drivers/net/axgbe/axgbe_rxtx.h
+++ b/drivers/net/axgbe/axgbe_rxtx.h
@@ -161,6 +161,10 @@ void axgbe_dev_enable_tx(struct rte_eth_dev *dev);
 void axgbe_dev_disable_tx(struct rte_eth_dev *dev);
 int axgbe_dev_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id);
 int axgbe_dev_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id);
+int axgbe_dev_rx_queue_start(struct rte_eth_dev *eth_dev, uint16_t queue_idx);
+int axgbe_dev_rx_queue_stop(struct rte_eth_dev *eth_dev, uint16_t queue_idx);
+int axgbe_dev_fw_version_get(struct rte_eth_dev *eth_dev,
+		char *fw_version, size_t fw_size);
 
 uint16_t axgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			 uint16_t nb_pkts);
-- 
2.17.1

Re: [dpdk-dev] [PATCH v1] net/axgbe: add support for utility APIs

[Somalapuram, Amaranath]

[AMD Public Use]

Acked-by: Somalapuram Amaranath <asomalap@amd.com>

-----Original Message-----
From: Sebastian, Selwin <Selwin.Sebastian@amd.com>
Sent: Monday, June 1, 2020 7:27 PM
To: dev@dpdk.org
Cc: Somalapuram, Amaranath <Amaranath.Somalapuram@amd.com>
Subject: [PATCH v1] net/axgbe: add support for utility APIs

From: Selwin Sebastian <selwin.sebastian@amd.com>

Add support for rx, tx queue utility APIs Add support for 'fw_revision_get' API

Signed-off-by: Selwin Sebastian <selwin.sebastian@amd.com>
---
 drivers/net/axgbe/axgbe_ethdev.c |  7 +++
 drivers/net/axgbe/axgbe_rxtx.c   | 89 ++++++++++++++++++++++++++++++++
 drivers/net/axgbe/axgbe_rxtx.h   |  4 ++
 3 files changed, 100 insertions(+)

diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index 867058845..e943d1dae 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -214,6 +214,11 @@ 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,
+.rx_queue_start       = axgbe_dev_rx_queue_start,
+.rx_queue_stop        = axgbe_dev_rx_queue_stop,
+.tx_queue_start       = axgbe_dev_tx_queue_start,
+.tx_queue_stop        = axgbe_dev_tx_queue_stop,
+.fw_version_get       = axgbe_dev_fw_version_get,
 };

 static int axgbe_phy_reset(struct axgbe_port *pdata) @@ -1006,6 +1011,8 @@ axgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 DEV_TX_OFFLOAD_IPV4_CKSUM  |
 DEV_TX_OFFLOAD_UDP_CKSUM   |
 DEV_TX_OFFLOAD_TCP_CKSUM;
+dev_info->dev_capa = RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP |
+RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP;

 if (pdata->hw_feat.rss) {
 dev_info->flow_type_rss_offloads = AXGBE_RSS_OFFLOAD; diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c index 30c467db7..495e07902 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -565,6 +565,95 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 return 0;
 }

+int axgbe_dev_tx_queue_start(struct rte_eth_dev *eth_dev,
+uint16_t queue_idx)
+{
+struct axgbe_port *pdata =
+(struct axgbe_port *)eth_dev->data->dev_private;
+int ret = 0;
+
+PMD_INIT_FUNC_TRACE();
+
+AXGMAC_MTL_IOWRITE_BITS(pdata, queue_idx,
+MTL_Q_TQOMR, TXQEN, MTL_Q_ENABLED);
+eth_dev->data->tx_queue_state[queue_idx] =
+RTE_ETH_QUEUE_STATE_STARTED;
+
+return ret;
+}
+
+int axgbe_dev_tx_queue_stop(struct rte_eth_dev *eth_dev,
+uint16_t queue_idx)
+{
+int ret = 0;
+struct axgbe_port *pdata =
+(struct axgbe_port *)eth_dev->data->dev_private;
+
+PMD_INIT_FUNC_TRACE();
+
+AXGMAC_MTL_IOWRITE_BITS(pdata, queue_idx, MTL_Q_TQOMR, TXQEN, 0);
+eth_dev->data->tx_queue_state[queue_idx] =
+RTE_ETH_QUEUE_STATE_STOPPED;
+
+return ret;
+}
+
+int axgbe_dev_rx_queue_start(struct rte_eth_dev *eth_dev,
+uint16_t queue_idx)
+{
+struct axgbe_port *pdata =
+(struct axgbe_port *)eth_dev->data->dev_private;
+unsigned int reg_val = 0;
+int ret = 0;
+
+PMD_INIT_FUNC_TRACE();
+
+reg_val |= (0x02 << (queue_idx << 1));
+AXGMAC_IOWRITE(pdata, MAC_RQC0R, reg_val);
+eth_dev->data->rx_queue_state[queue_idx] =
+RTE_ETH_QUEUE_STATE_STARTED;
+
+return ret;
+}
+
+int axgbe_dev_rx_queue_stop(struct rte_eth_dev *eth_dev,
+uint16_t queue_idx)
+{
+int ret = 0;
+struct axgbe_port *pdata =
+(struct axgbe_port *)eth_dev->data->dev_private;
+struct axgbe_rx_queue *rxq;
+
+PMD_INIT_FUNC_TRACE();
+
+AXGMAC_IOWRITE(pdata, MAC_RQC0R, 0);
+rxq = eth_dev->data->rx_queues[queue_idx];
+
+/* Disable Rx DMA channel */
+AXGMAC_DMA_IOWRITE_BITS(rxq, DMA_CH_RCR, SR, 0);
+eth_dev->data->rx_queue_state[queue_idx] =
+RTE_ETH_QUEUE_STATE_STOPPED;
+
+return ret;
+}
+
+int axgbe_dev_fw_version_get(struct rte_eth_dev *eth_dev,
+char *fw_version, size_t fw_size)
+{
+struct axgbe_port *pdata;
+struct axgbe_hw_features *hw_feat;
+char fw_ver[32];
+
+pdata = (struct axgbe_port *)eth_dev->data->dev_private;
+hw_feat = &pdata->hw_feat;
+
+if (fw_version == NULL || fw_size <= 0)
+return -EINVAL;
+
+snprintf(fw_version, sizeof(fw_ver), "%d.%d.%d",
+AXGMAC_GET_BITS(hw_feat->version, MAC_VR, USERVER),
+AXGMAC_GET_BITS(hw_feat->version, MAC_VR, DEVID),
+AXGMAC_GET_BITS(hw_feat->version, MAC_VR, SNPSVER));
+
+return 0;
+}
+
 static void axgbe_txq_prepare_tx_stop(struct axgbe_port *pdata,
       unsigned int queue)
 {
diff --git a/drivers/net/axgbe/axgbe_rxtx.h b/drivers/net/axgbe/axgbe_rxtx.h index f2fbe9299..3e05e0e6f 100644
--- a/drivers/net/axgbe/axgbe_rxtx.h
+++ b/drivers/net/axgbe/axgbe_rxtx.h
@@ -161,6 +161,10 @@ void axgbe_dev_enable_tx(struct rte_eth_dev *dev);  void axgbe_dev_disable_tx(struct rte_eth_dev *dev);  int axgbe_dev_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id);  int axgbe_dev_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id);
+int axgbe_dev_rx_queue_start(struct rte_eth_dev *eth_dev, uint16_t
+queue_idx); int axgbe_dev_rx_queue_stop(struct rte_eth_dev *eth_dev,
+uint16_t queue_idx); int axgbe_dev_fw_version_get(struct rte_eth_dev *eth_dev,
+char *fw_version, size_t fw_size);

 uint16_t axgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
  uint16_t nb_pkts);
--
2.17.1

Re: [dpdk-dev] [PATCH v1] net/axgbe: add support for utility APIs

[Ferruh Yigit]

On 6/1/2020 2:57 PM, selwin.sebastian@amd.com wrote:
> From: Selwin Sebastian <selwin.sebastian@amd.com>
> 
> Add support for rx, tx queue utility APIs
> Add support for 'fw_revision_get' API
> 
> Signed-off-by: Selwin Sebastian <selwin.sebastian@amd.com>

Hi Selwin,

Can you please split this patch into three patches,
1) rx/tx queue start/stop
Please add "Queue start/stop" support in 'axgbe.ini' in this patch
2) Fw version get, and announce "FW version" in .ini file
3) Runtime Rx/Tx queue setup capability, and announce "Runtime Rx queue setup" &
"Runtime Tx queue setup" in .ini file

<...>

> @@ -565,6 +565,95 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
>  	return 0;
>  }
>  
> +int axgbe_dev_tx_queue_start(struct rte_eth_dev *eth_dev,
> +		uint16_t queue_idx)
> +{

According coding convention return type should be in above line, this file has
both syntax which is worst. Is there any reason to comply to coding convention
sometimes but not comply other times?