diff mbox

[dpdk-dev,v4,28/41] net/dpaa: add support for link status update

Message ID 20170909112132.13936-29-shreyansh.jain@nxp.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Shreyansh Jain Sept. 9, 2017, 11:21 a.m. UTC 
  Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 doc/guides/nics/features/dpaa.ini |  1 +
 drivers/net/dpaa/dpaa_ethdev.c    | 42 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

Comments

Ferruh Yigit Sept. 18, 2017, 2:56 p.m. UTC | #1 
On 9/9/2017 12:21 PM, Shreyansh Jain wrote:
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>

<...>

> +static int dpaa_eth_link_update(struct rte_eth_dev *dev,
> +				int wait_to_complete __rte_unused)
> +{
> +	struct dpaa_if *dpaa_intf = dev->data->dev_private;
> +	struct rte_eth_link *link = &dev->data->dev_link;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (dpaa_intf->fif->mac_type == fman_mac_1g)
> +		link->link_speed = 1000;
> +	else if (dpaa_intf->fif->mac_type == fman_mac_10g)
> +		link->link_speed = 10000;
> +	else
> +		DPAA_PMD_ERR("invalid link_speed: %s, %d",
> +			     dpaa_intf->name, dpaa_intf->fif->mac_type);
> +
> +	link->link_status = dpaa_intf->valid;
> +	link->link_duplex = ETH_LINK_FULL_DUPLEX;
> +	link->link_autoneg = ETH_LINK_AUTONEG;

Shouldn't this function go and get link information from hardware?

> +	return 0;
> +}
> +
>  static
>  int dpaa_eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
>  			    uint16_t nb_desc __rte_unused,
> @@ -216,6 +238,22 @@ static void dpaa_eth_tx_queue_release(void *txq __rte_unused)
>  	PMD_INIT_FUNC_TRACE();
>  }
>  
> +static int dpaa_link_down(struct rte_eth_dev *dev)
> +{
> +	PMD_INIT_FUNC_TRACE();
> +
> +	dpaa_eth_dev_stop(dev);

Drivers tend to do revers, make link down on device stop. Just to double
check if stop() is intended for link down.

> +	return 0;
> +}
> +
> +static int dpaa_link_up(struct rte_eth_dev *dev)
> +{
> +	PMD_INIT_FUNC_TRACE();
> +
> +	dpaa_eth_dev_start(dev);
> +	return 0;
> +}

<...>
Shreyansh Jain Sept. 21, 2017, 1:09 p.m. UTC | #2 
On Monday 18 September 2017 08:26 PM, Ferruh Yigit wrote:
> On 9/9/2017 12:21 PM, Shreyansh Jain wrote:
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> 
> <...>
> 
>> +static int dpaa_eth_link_update(struct rte_eth_dev *dev,
>> +				int wait_to_complete __rte_unused)
>> +{
>> +	struct dpaa_if *dpaa_intf = dev->data->dev_private;
>> +	struct rte_eth_link *link = &dev->data->dev_link;
>> +
>> +	PMD_INIT_FUNC_TRACE();
>> +
>> +	if (dpaa_intf->fif->mac_type == fman_mac_1g)
>> +		link->link_speed = 1000;
>> +	else if (dpaa_intf->fif->mac_type == fman_mac_10g)
>> +		link->link_speed = 10000;
>> +	else
>> +		DPAA_PMD_ERR("invalid link_speed: %s, %d",
>> +			     dpaa_intf->name, dpaa_intf->fif->mac_type);
>> +
>> +	link->link_status = dpaa_intf->valid;
>> +	link->link_duplex = ETH_LINK_FULL_DUPLEX;
>> +	link->link_autoneg = ETH_LINK_AUTONEG;
> 
> Shouldn't this function go and get link information from hardware?

Our currently hardware interfaces don't support these operations 
explicitly. For the "fman_mac_1g" and "fman_mac_10g", these are the 
default sets which are exposing.
Overtime, we will get more such interfaces exposed from Linux kernel to 
Fman library and update this code.

> 
>> +	return 0;
>> +}
>> +
>>   static
>>   int dpaa_eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
>>   			    uint16_t nb_desc __rte_unused,
>> @@ -216,6 +238,22 @@ static void dpaa_eth_tx_queue_release(void *txq __rte_unused)
>>   	PMD_INIT_FUNC_TRACE();
>>   }
>>   
>> +static int dpaa_link_down(struct rte_eth_dev *dev)
>> +{
>> +	PMD_INIT_FUNC_TRACE();
>> +
>> +	dpaa_eth_dev_stop(dev);
> 
> Drivers tend to do revers, make link down on device stop. Just to double
> check if stop() is intended for link down.

fman_if_disable_rx is equivalent to "link down" as well as stop (because 
it flushes the queues). That is why these APIs are linked.

> 
>> +	return 0;
>> +}
>> +
>> +static int dpaa_link_up(struct rte_eth_dev *dev)
>> +{
>> +	PMD_INIT_FUNC_TRACE();
>> +
>> +	dpaa_eth_dev_start(dev);
>> +	return 0;
>> +}
> 
> <...>
>
diff mbox

Patch

diff --git a/doc/guides/nics/features/dpaa.ini b/doc/guides/nics/features/dpaa.ini
index e62812c..132f94b 100644
--- a/doc/guides/nics/features/dpaa.ini
+++ b/doc/guides/nics/features/dpaa.ini
@@ -4,6 +4,7 @@ 
 ; Refer to default.ini for the full list of available PMD features.
 ;
 [Features]
+Link status          = Y
 Jumbo frame          = Y
 MTU update           = Y
 ARMv8                = Y
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index d0bab36..75fded2 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -142,6 +142,28 @@  static void dpaa_eth_dev_close(struct rte_eth_dev *dev)
 	dpaa_eth_dev_stop(dev);
 }
 
+static int dpaa_eth_link_update(struct rte_eth_dev *dev,
+				int wait_to_complete __rte_unused)
+{
+	struct dpaa_if *dpaa_intf = dev->data->dev_private;
+	struct rte_eth_link *link = &dev->data->dev_link;
+
+	PMD_INIT_FUNC_TRACE();
+
+	if (dpaa_intf->fif->mac_type == fman_mac_1g)
+		link->link_speed = 1000;
+	else if (dpaa_intf->fif->mac_type == fman_mac_10g)
+		link->link_speed = 10000;
+	else
+		DPAA_PMD_ERR("invalid link_speed: %s, %d",
+			     dpaa_intf->name, dpaa_intf->fif->mac_type);
+
+	link->link_status = dpaa_intf->valid;
+	link->link_duplex = ETH_LINK_FULL_DUPLEX;
+	link->link_autoneg = ETH_LINK_AUTONEG;
+	return 0;
+}
+
 static
 int dpaa_eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 			    uint16_t nb_desc __rte_unused,
@@ -216,6 +238,22 @@  static void dpaa_eth_tx_queue_release(void *txq __rte_unused)
 	PMD_INIT_FUNC_TRACE();
 }
 
+static int dpaa_link_down(struct rte_eth_dev *dev)
+{
+	PMD_INIT_FUNC_TRACE();
+
+	dpaa_eth_dev_stop(dev);
+	return 0;
+}
+
+static int dpaa_link_up(struct rte_eth_dev *dev)
+{
+	PMD_INIT_FUNC_TRACE();
+
+	dpaa_eth_dev_start(dev);
+	return 0;
+}
+
 static struct eth_dev_ops dpaa_devops = {
 	.dev_configure		  = dpaa_eth_dev_configure,
 	.dev_start		  = dpaa_eth_dev_start,
@@ -226,7 +264,11 @@  static struct eth_dev_ops dpaa_devops = {
 	.tx_queue_setup		  = dpaa_eth_tx_queue_setup,
 	.rx_queue_release	  = dpaa_eth_rx_queue_release,
 	.tx_queue_release	  = dpaa_eth_tx_queue_release,
+
+	.link_update		  = dpaa_eth_link_update,
 	.mtu_set		  = dpaa_mtu_set,
+	.dev_set_link_down	  = dpaa_link_down,
+	.dev_set_link_up	  = dpaa_link_up,
 };
 
 /* Initialise an Rx FQ */