[dpdk-dev,24/29] net/ixgbe/base: add EEE support for DNL-controlled PHYs

Message ID 1480833100-48545-24-git-send-email-wei.dai@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
checkpatch/checkpatch success coding style OK

Commit Message

Wei Dai Dec. 4, 2016, 6:31 a.m. UTC
  This patch adds EEE support for DNL-controlled PHYs. Because DNL
does not indicate EEE capability or status, this patch simply
assumes that it is supported. As soon as there is a DNL-supported
PHY that does not support EEE, there will be defects in this area
because the driver will not report the EEE status correctly.
This also deletes some now-unused definitions from an earlier
Marvell PHY implementation and combines a device ID check into a
switch statement.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_type.h |  8 --------
 drivers/net/ixgbe/base/ixgbe_x550.c | 15 +++++++--------
 2 files changed, 7 insertions(+), 16 deletions(-)
  

Comments

Ferruh Yigit Dec. 5, 2016, 7:40 p.m. UTC | #1
On 12/4/2016 6:31 AM, Wei Dai wrote:
> This patch adds EEE support for DNL-controlled PHYs. Because DNL

What is DNL?

> does not indicate EEE capability or status, this patch simply
> assumes that it is supported. As soon as there is a DNL-supported
> PHY that does not support EEE, there will be defects in this area
> because the driver will not report the EEE status correctly.
> This also deletes some now-unused definitions from an earlier
> Marvell PHY implementation and combines a device ID check into a
> switch statement.
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> ---
>  drivers/net/ixgbe/base/ixgbe_type.h |  8 --------
>  drivers/net/ixgbe/base/ixgbe_x550.c | 15 +++++++--------
>  2 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
> index 9ec17a9..f1761a3 100644
> --- a/drivers/net/ixgbe/base/ixgbe_type.h
> +++ b/drivers/net/ixgbe/base/ixgbe_type.h
> @@ -3720,14 +3720,6 @@ enum ixgbe_fc_mode {
>  	ixgbe_fc_default
>  };
>  
> -/* Master/slave control */
> -enum ixgbe_ms_type {
> -	ixgbe_ms_hw_default = 0,
> -	ixgbe_ms_force_master,
> -	ixgbe_ms_force_slave,
> -	ixgbe_ms_auto
> -};
> -

This seems not related to this patchset, also patch 15/29 has [1] again
seems unrelated to that patch, does it make sense to make these a
separate patch?

[1]
"
@@ -4046,8 +4112,8 @@ struct ixgbe_phy_info {
 	bool reset_disable;
 	ixgbe_autoneg_advertised autoneg_advertised;
 	ixgbe_link_speed speeds_supported;
-	enum ixgbe_ms_type ms_type;
-	enum ixgbe_ms_type original_ms_type;
  
Wei Dai Dec. 21, 2016, 10:17 a.m. UTC | #2
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, December 6, 2016 3:41 AM
> To: Dai, Wei <wei.dai@intel.com>; Zhang, Helin <helin.zhang@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 24/29] net/ixgbe/base: add EEE support for
> DNL-controlled PHYs
> 
> On 12/4/2016 6:31 AM, Wei Dai wrote:
> > This patch adds EEE support for DNL-controlled PHYs. Because DNL
> 
> What is DNL?
DNL is short for name of some method to control Marvel PHY.
This git log message will be revised in v2 patch set.

> 
> > does not indicate EEE capability or status, this patch simply assumes
> > that it is supported. As soon as there is a DNL-supported PHY that
> > does not support EEE, there will be defects in this area because the
> > driver will not report the EEE status correctly.
> > This also deletes some now-unused definitions from an earlier Marvell
> > PHY implementation and combines a device ID check into a switch
> > statement.
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > ---
> >  drivers/net/ixgbe/base/ixgbe_type.h |  8 --------
> > drivers/net/ixgbe/base/ixgbe_x550.c | 15 +++++++--------
> >  2 files changed, 7 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_type.h
> > b/drivers/net/ixgbe/base/ixgbe_type.h
> > index 9ec17a9..f1761a3 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_type.h
> > +++ b/drivers/net/ixgbe/base/ixgbe_type.h
> > @@ -3720,14 +3720,6 @@ enum ixgbe_fc_mode {
> >  	ixgbe_fc_default
> >  };
> >
> > -/* Master/slave control */
> > -enum ixgbe_ms_type {
> > -	ixgbe_ms_hw_default = 0,
> > -	ixgbe_ms_force_master,
> > -	ixgbe_ms_force_slave,
> > -	ixgbe_ms_auto
> > -};
> > -
> 
> This seems not related to this patchset, also patch 15/29 has [1] again seems
> unrelated to that patch, does it make sense to make these a separate patch?
Yes, in v2 patch set, removing of above enum type will be in a separate one. 

> 
> [1]
> "
> @@ -4046,8 +4112,8 @@ struct ixgbe_phy_info {
>  	bool reset_disable;
>  	ixgbe_autoneg_advertised autoneg_advertised;
>  	ixgbe_link_speed speeds_supported;
> -	enum ixgbe_ms_type ms_type;
> -	enum ixgbe_ms_type original_ms_type;
> 
>
  

Patch

diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
index 9ec17a9..f1761a3 100644
--- a/drivers/net/ixgbe/base/ixgbe_type.h
+++ b/drivers/net/ixgbe/base/ixgbe_type.h
@@ -3720,14 +3720,6 @@  enum ixgbe_fc_mode {
 	ixgbe_fc_default
 };
 
-/* Master/slave control */
-enum ixgbe_ms_type {
-	ixgbe_ms_hw_default = 0,
-	ixgbe_ms_force_master,
-	ixgbe_ms_force_slave,
-	ixgbe_ms_auto
-};
-
 /* Smart Speed Settings */
 #define IXGBE_SMARTSPEED_MAX_RETRIES	3
 enum ixgbe_smart_speed {
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index 49b59e7..2aaed6b 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -896,19 +896,18 @@  s32 ixgbe_init_ops_X550EM_a(struct ixgbe_hw *hw)
 		break;
 	}
 
-	if ((hw->device_id == IXGBE_DEV_ID_X550EM_A_1G_T) ||
-		(hw->device_id == IXGBE_DEV_ID_X550EM_A_1G_T_L)) {
+	switch (hw->device_id) {
+	case IXGBE_DEV_ID_X550EM_A_1G_T:
+	case IXGBE_DEV_ID_X550EM_A_1G_T_L:
 		mac->ops.fc_autoneg = ixgbe_fc_autoneg_sgmii_x550em_a;
 		mac->ops.setup_fc = ixgbe_fc_autoneg_fw;
-	}
-
-	switch (hw->device_id) {
-	case IXGBE_DEV_ID_X550EM_A_KR:
-	case IXGBE_DEV_ID_X550EM_A_KR_L:
 		mac->ops.setup_eee = ixgbe_setup_eee_fw;
+		hw->phy.eee_speeds_supported = IXGBE_LINK_SPEED_100_FULL |
+					       IXGBE_LINK_SPEED_1GB_FULL;
+		hw->phy.eee_speeds_advertised = hw->phy.eee_speeds_supported;
 		break;
 	default:
-		mac->ops.setup_eee = NULL;
+		break;
 	}
 
 	return ret_val;