[dpdk-dev] [PATCH 0/7] update ixgbe shared code to version 2017-01-05

[dpdk-dev] [PATCH 0/7] update ixgbe shared code to version 2017-01-05

[Wei Dai]

This patch set updates ixgbe shared code to cid-ixgbe.2017.01.05.tar.gz .
The shared codes are in drivers/net/ixgbe/base/ and are developed by 
another Intel team.

This patch add more supports to some Intel Broadwell-DE platforms.
A new device id and two PHY types are introduced and also 
add some debug traces to help debugging.

Wei Dai (7):
  net/ixgbe/base: support Broadwell-DE XFI backplane
  net/ixgbe/base: remove a compiler warning
  net/ixgbe/base: configure speeds for KR/KX backplane
  net/ixgbe/base: support Marvell 1000BASE-T PHYs
  net/ixgbe/base: use clause 22 MDIO functions for Marvell PHYs
  net/ixgbe/base: add some debug traces
  net/ixgbe/base: update version of shared code

 drivers/net/ixgbe/base/README         |  2 +-
 drivers/net/ixgbe/base/ixgbe_api.c    |  1 +
 drivers/net/ixgbe/base/ixgbe_common.c |  9 +++-
 drivers/net/ixgbe/base/ixgbe_phy.c    | 20 +++++++--
 drivers/net/ixgbe/base/ixgbe_type.h   |  3 ++
 drivers/net/ixgbe/base/ixgbe_x540.c   | 11 ++++-
 drivers/net/ixgbe/base/ixgbe_x550.c   | 77 ++++++++++++++++++++++++++++++-----
 7 files changed, 104 insertions(+), 19 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/7] net/ixgbe/base: support Broadwell-DE XFI backplane

[Wei Dai]

This patch adds initial support for a Braodwell-DE XFI backplane
interface. The XFI backplane requires a custom tuned link.
Hardware/Firmware owns the link config for XF backplane and software
must not interfere with it.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_api.c  |  1 +
 drivers/net/ixgbe/base/ixgbe_type.h |  2 ++
 drivers/net/ixgbe/base/ixgbe_x550.c | 11 +++++++++++
 3 files changed, 14 insertions(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_api.c b/drivers/net/ixgbe/base/ixgbe_api.c
index 0ddafcb..270a97d 100644
--- a/drivers/net/ixgbe/base/ixgbe_api.c
+++ b/drivers/net/ixgbe/base/ixgbe_api.c
@@ -205,6 +205,7 @@ s32 ixgbe_set_mac_type(struct ixgbe_hw *hw)
 	case IXGBE_DEV_ID_X550EM_X_10G_T:
 	case IXGBE_DEV_ID_X550EM_X_1G_T:
 	case IXGBE_DEV_ID_X550EM_X_SFP:
+	case IXGBE_DEV_ID_X550EM_X_XFI:
 		hw->mac.type = ixgbe_mac_X550EM_x;
 		hw->mvals = ixgbe_mvals_X550EM_x;
 		break;
diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
index b609f6e..dea210e 100644
--- a/drivers/net/ixgbe/base/ixgbe_type.h
+++ b/drivers/net/ixgbe/base/ixgbe_type.h
@@ -146,6 +146,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #define IXGBE_DEV_ID_X550EM_X_SFP		0x15AC
 #define IXGBE_DEV_ID_X550EM_X_10G_T		0x15AD
 #define IXGBE_DEV_ID_X550EM_X_1G_T		0x15AE
+#define IXGBE_DEV_ID_X550EM_X_XFI		0x15B0
 #define IXGBE_DEV_ID_X550_VF_HV			0x1564
 #define IXGBE_DEV_ID_X550_VF			0x1565
 #define IXGBE_DEV_ID_X550EM_A_VF		0x15C5
@@ -3644,6 +3645,7 @@ enum ixgbe_phy_type {
 	ixgbe_phy_aq,
 	ixgbe_phy_x550em_kr,
 	ixgbe_phy_x550em_kx4,
+	ixgbe_phy_x550em_xfi,
 	ixgbe_phy_x550em_ext_t,
 	ixgbe_phy_cu_unknown,
 	ixgbe_phy_qt,
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index 782ca91..2f7f446 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -450,6 +450,9 @@ STATIC s32 ixgbe_identify_phy_x550em(struct ixgbe_hw *hw)
 	case IXGBE_DEV_ID_X550EM_X_KX4:
 		hw->phy.type = ixgbe_phy_x550em_kx4;
 		break;
+	case IXGBE_DEV_ID_X550EM_X_XFI:
+		hw->phy.type = ixgbe_phy_x550em_xfi;
+		break;
 	case IXGBE_DEV_ID_X550EM_X_KR:
 	case IXGBE_DEV_ID_X550EM_A_KR:
 	case IXGBE_DEV_ID_X550EM_A_KR_L:
@@ -1573,6 +1576,7 @@ enum ixgbe_media_type ixgbe_get_media_type_X550em(struct ixgbe_hw *hw)
 	switch (hw->device_id) {
 	case IXGBE_DEV_ID_X550EM_X_KR:
 	case IXGBE_DEV_ID_X550EM_X_KX4:
+	case IXGBE_DEV_ID_X550EM_X_XFI:
 	case IXGBE_DEV_ID_X550EM_A_KR:
 	case IXGBE_DEV_ID_X550EM_A_KR_L:
 		media_type = ixgbe_media_type_backplane;
@@ -2395,6 +2399,12 @@ s32 ixgbe_init_phy_ops_X550em(struct ixgbe_hw *hw)
 		phy->ops.read_reg = ixgbe_read_phy_reg_x550em;
 		phy->ops.write_reg = ixgbe_write_phy_reg_x550em;
 		break;
+	case ixgbe_phy_x550em_xfi:
+		/* link is managed by HW */
+		phy->ops.setup_link = NULL;
+		phy->ops.read_reg = ixgbe_read_phy_reg_x550em;
+		phy->ops.write_reg = ixgbe_write_phy_reg_x550em;
+		break;
 	case ixgbe_phy_x550em_ext_t:
 		/* If internal link mode is XFI, then setup iXFI internal link,
 		 * else setup KR now.
@@ -3669,6 +3679,7 @@ u32 ixgbe_get_supported_physical_layer_X550em(struct ixgbe_hw *hw)
 
 	switch (hw->phy.type) {
 	case ixgbe_phy_x550em_kr:
+	case ixgbe_phy_x550em_xfi:
 		physical_layer = IXGBE_PHYSICAL_LAYER_10GBASE_KR |
 				 IXGBE_PHYSICAL_LAYER_1000BASE_KX;
 		break;
-- 
2.7.4

Re: [dpdk-dev] [PATCH 1/7] net/ixgbe/base: support Broadwell-DE XFI backplane

[Ferruh Yigit]

On 1/10/2017 3:45 PM, Wei Dai wrote:
> This patch adds initial support for a Braodwell-DE XFI backplane
> interface. The XFI backplane requires a custom tuned link.
> Hardware/Firmware owns the link config for XF backplane and software
> must not interfere with it.

Does it make sense to announce this support in release notes?

> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
<...>

Re: [dpdk-dev] [PATCH 1/7] net/ixgbe/base: support Broadwell-DE XFI backplane

[Dai, Wei]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, January 11, 2017 11:38 PM
> To: Dai, Wei <wei.dai@intel.com>; dev@dpdk.org
> Cc: Zhang, Helin <helin.zhang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 1/7] net/ixgbe/base: support Broadwell-DE XFI
> backplane
> 
> On 1/10/2017 3:45 PM, Wei Dai wrote:
> > This patch adds initial support for a Braodwell-DE XFI backplane
> > interface. The XFI backplane requires a custom tuned link.
> > Hardware/Firmware owns the link config for XF backplane and software
> > must not interfere with it.
> 
> Does it make sense to announce this support in release notes?
This is just an initial support and need to be further completed and optimized,
So I don't think it is suitable to announce in this release note.
Maybe we can do that in next release 17.05 .

> 
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> <...>

[dpdk-dev] [PATCH 2/7] net/ixgbe/base: remove a compiler warning

[Wei Dai]

This patch fixes a compiler warning.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_common.c b/drivers/net/ixgbe/base/ixgbe_common.c
index 18bb18c..0dd33b8 100644
--- a/drivers/net/ixgbe/base/ixgbe_common.c
+++ b/drivers/net/ixgbe/base/ixgbe_common.c
@@ -1150,7 +1150,7 @@ s32 ixgbe_init_led_link_act_generic(struct ixgbe_hw *hw)
 {
 	struct ixgbe_mac_info *mac = &hw->mac;
 	u32 led_reg, led_mode;
-	u16 i;
+	u8 i;
 
 	led_reg = IXGBE_READ_REG(hw, IXGBE_LEDCTL);
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH 2/7] net/ixgbe/base: remove a compiler warning

[Ferruh Yigit]

On 1/10/2017 3:45 PM, Wei Dai wrote:
> This patch fixes a compiler warning.

I guess compiler warning is because of "mac->led_link_act = i;", since
led_link_act is 8bits..

But can you please add the compile warning here, to make it more clear
to understand what is the issue.

> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> ---
>  drivers/net/ixgbe/base/ixgbe_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/base/ixgbe_common.c b/drivers/net/ixgbe/base/ixgbe_common.c
> index 18bb18c..0dd33b8 100644
> --- a/drivers/net/ixgbe/base/ixgbe_common.c
> +++ b/drivers/net/ixgbe/base/ixgbe_common.c
> @@ -1150,7 +1150,7 @@ s32 ixgbe_init_led_link_act_generic(struct ixgbe_hw *hw)
>  {
>  	struct ixgbe_mac_info *mac = &hw->mac;
>  	u32 led_reg, led_mode;
> -	u16 i;
> +	u8 i;
>  
>  	led_reg = IXGBE_READ_REG(hw, IXGBE_LEDCTL);
>  
> 

Re: [dpdk-dev] [PATCH 2/7] net/ixgbe/base: remove a compiler warning

[Dai, Wei]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, January 11, 2017 11:38 PM
> To: Dai, Wei <wei.dai@intel.com>; dev@dpdk.org
> Cc: Zhang, Helin <helin.zhang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 2/7] net/ixgbe/base: remove a compiler warning
> 
> On 1/10/2017 3:45 PM, Wei Dai wrote:
> > This patch fixes a compiler warning.
> 
> I guess compiler warning is because of "mac->led_link_act = i;", since
> led_link_act is 8bits..
> 
> But can you please add the compile warning here, to make it more clear to
> understand what is the issue.

I have compared the generated ixgbe_common.o both from "u16 i" and "u8 I" .
Two .o files are identical. 
Whether the variable i is defined as u8 or u16,  there is no compile warning. 
The version of gcc version is 6.3.1 20161221 (Red Hat 6.3.1-1) (GCC).
If i is u16, mac->led_link_act = i; should generate a warning, but there is no.
It is a bit weird .
As you know, this code is provided by another Intel team.
I think that it will generate a warning when they build it with their C compiler.
Anyway, to define i as u8 is better than u16 and we had better keep aligned with them.

> 
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > ---
> >  drivers/net/ixgbe/base/ixgbe_common.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_common.c
> > b/drivers/net/ixgbe/base/ixgbe_common.c
> > index 18bb18c..0dd33b8 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_common.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_common.c
> > @@ -1150,7 +1150,7 @@ s32 ixgbe_init_led_link_act_generic(struct
> > ixgbe_hw *hw)  {
> >  	struct ixgbe_mac_info *mac = &hw->mac;
> >  	u32 led_reg, led_mode;
> > -	u16 i;
> > +	u8 i;
> >
> >  	led_reg = IXGBE_READ_REG(hw, IXGBE_LEDCTL);
> >
> >

Re: [dpdk-dev] [PATCH 2/7] net/ixgbe/base: remove a compiler warning

[Ferruh Yigit]

On 1/12/2017 2:08 PM, Dai, Wei wrote:
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Wednesday, January 11, 2017 11:38 PM
>> To: Dai, Wei <wei.dai@intel.com>; dev@dpdk.org
>> Cc: Zhang, Helin <helin.zhang@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH 2/7] net/ixgbe/base: remove a compiler warning
>>
>> On 1/10/2017 3:45 PM, Wei Dai wrote:
>>> This patch fixes a compiler warning.
>>
>> I guess compiler warning is because of "mac->led_link_act = i;", since
>> led_link_act is 8bits..
>>
>> But can you please add the compile warning here, to make it more clear to
>> understand what is the issue.
> 
> I have compared the generated ixgbe_common.o both from "u16 i" and "u8 I" .
> Two .o files are identical. 
> Whether the variable i is defined as u8 or u16,  there is no compile warning. 
> The version of gcc version is 6.3.1 20161221 (Red Hat 6.3.1-1) (GCC).
> If i is u16, mac->led_link_act = i; should generate a warning, but there is no.
> It is a bit weird .
> As you know, this code is provided by another Intel team.
> I think that it will generate a warning when they build it with their C compiler.
> Anyway, to define i as u8 is better than u16 and we had better keep aligned with them.

Agreed, u8 is better.

> 
>>
>>>
>>> Signed-off-by: Wei Dai <wei.dai@intel.com>
>>> ---
>>>  drivers/net/ixgbe/base/ixgbe_common.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ixgbe/base/ixgbe_common.c
>>> b/drivers/net/ixgbe/base/ixgbe_common.c
>>> index 18bb18c..0dd33b8 100644
>>> --- a/drivers/net/ixgbe/base/ixgbe_common.c
>>> +++ b/drivers/net/ixgbe/base/ixgbe_common.c
>>> @@ -1150,7 +1150,7 @@ s32 ixgbe_init_led_link_act_generic(struct
>>> ixgbe_hw *hw)  {
>>>  	struct ixgbe_mac_info *mac = &hw->mac;
>>>  	u32 led_reg, led_mode;
>>> -	u16 i;
>>> +	u8 i;
>>>
>>>  	led_reg = IXGBE_READ_REG(hw, IXGBE_LEDCTL);
>>>
>>>
> 

[dpdk-dev] [PATCH 3/7] net/ixgbe/base: configure speeds for KR/KX backplane

[Wei Dai]

This patch ensures that the advertised link speeds are configured
for KR/KX backplane on some new platform. Without this patch the
link remains at 1G when resuming from low power after being
downshifted by LPLU(Low Power Link Up).
This patch ensures that the advertise speeds are not changed
for 2.5G configurations.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_x550.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index 2f7f446..b23d479 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -2627,12 +2627,11 @@ s32 ixgbe_init_ext_t_x550em(struct ixgbe_hw *hw)
 /**
  *  ixgbe_setup_kr_x550em - Configure the KR PHY.
  *  @hw: pointer to hardware structure
- *
- *  Configures the integrated KR PHY for X550EM_x.
  **/
 s32 ixgbe_setup_kr_x550em(struct ixgbe_hw *hw)
 {
-	if (hw->mac.type != ixgbe_mac_X550EM_x)
+	/* leave link alone for 2.5G */
+	if (hw->phy.autoneg_advertised & IXGBE_LINK_SPEED_2_5GB_FULL)
 		return IXGBE_SUCCESS;
 
 	return ixgbe_setup_kr_speed_x550em(hw, hw->phy.autoneg_advertised);
-- 
2.7.4

[dpdk-dev] [PATCH 4/7] net/ixgbe/base: support Marvell 1000BASE-T PHYs

[Wei Dai]

This patch adds initial support for Marvell 1000BASE-T PHYs
on Broadwell-DE.
Firmware owns the link config for Marvell PHYs on Broadwell-DE,
software should not touch it.
Also Broadwell-DE is not capable of speeds lower than 1Gb.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_phy.c  | 4 ++++
 drivers/net/ixgbe/base/ixgbe_type.h | 1 +
 drivers/net/ixgbe/base/ixgbe_x550.c | 8 ++++++++
 3 files changed, 13 insertions(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index 5ad4dfe..e0d7125 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -472,6 +472,10 @@ enum ixgbe_phy_type ixgbe_get_phy_type_from_id(u32 phy_id)
 	case X557_PHY_ID2:
 		phy_type = ixgbe_phy_x550em_ext_t;
 		break;
+	case IXGBE_M88E1500_E_PHY_ID:
+	case IXGBE_M88E1543_E_PHY_ID:
+		phy_type = ixgbe_phy_ext_1g_t;
+		break;
 	default:
 		phy_type = ixgbe_phy_unknown;
 		break;
diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
index dea210e..bb1f85b 100644
--- a/drivers/net/ixgbe/base/ixgbe_type.h
+++ b/drivers/net/ixgbe/base/ixgbe_type.h
@@ -3647,6 +3647,7 @@ enum ixgbe_phy_type {
 	ixgbe_phy_x550em_kx4,
 	ixgbe_phy_x550em_xfi,
 	ixgbe_phy_x550em_ext_t,
+	ixgbe_phy_ext_1g_t,
 	ixgbe_phy_cu_unknown,
 	ixgbe_phy_qt,
 	ixgbe_phy_xaui,
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index b23d479..1fd7ffb 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1975,6 +1975,7 @@ s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
 			*speed = IXGBE_LINK_SPEED_10GB_FULL;
 	} else {
 		switch (hw->phy.type) {
+		case ixgbe_phy_ext_1g_t:
 		case ixgbe_phy_sgmii:
 			*speed = IXGBE_LINK_SPEED_1GB_FULL;
 			break;
@@ -2399,6 +2400,10 @@ s32 ixgbe_init_phy_ops_X550em(struct ixgbe_hw *hw)
 		phy->ops.read_reg = ixgbe_read_phy_reg_x550em;
 		phy->ops.write_reg = ixgbe_write_phy_reg_x550em;
 		break;
+	case ixgbe_phy_ext_1g_t:
+		/* link is managed by FW */
+		phy->ops.setup_link = NULL;
+		break;
 	case ixgbe_phy_x550em_xfi:
 		/* link is managed by HW */
 		phy->ops.setup_link = NULL;
@@ -3706,6 +3711,9 @@ u32 ixgbe_get_supported_physical_layer_X550em(struct ixgbe_hw *hw)
 	case ixgbe_phy_sgmii:
 		physical_layer = IXGBE_PHYSICAL_LAYER_1000BASE_KX;
 		break;
+	case ixgbe_phy_ext_1g_t:
+		physical_layer |= IXGBE_PHYSICAL_LAYER_1000BASE_T;
+		break;
 	default:
 		break;
 	}
-- 
2.7.4

[dpdk-dev] [PATCH 5/7] net/ixgbe/base: use clause 22 MDIO functions for Marvell PHYs

[Wei Dai]

This patch sets the MDIO(Management Data Input/Output Interface)
read/write function ponters for Broadwell+Marvell PHYs to use
the clause 22 functions. Marvell PHYs do not support clause 45.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_x550.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index 1fd7ffb..e15054b 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -2373,6 +2373,10 @@ s32 ixgbe_init_phy_ops_X550em(struct ixgbe_hw *hw)
 		/* set up for CS4227 usage */
 		hw->phy.phy_semaphore_mask = IXGBE_GSSR_SHARED_I2C_SM;
 		break;
+	case IXGBE_DEV_ID_X550EM_X_1G_T:
+		phy->ops.read_reg_mdi = ixgbe_read_phy_reg_mdi_22;
+		phy->ops.write_reg_mdi = ixgbe_write_phy_reg_mdi_22;
+		break;
 	default:
 		break;
 	}
-- 
2.7.4

[dpdk-dev] [PATCH 6/7] net/ixgbe/base: add some debug traces

[Wei Dai]

This patch adds some traces in the reset_hw logic
and semaphore acquisition logic to help debugging.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_common.c |  7 ++++-
 drivers/net/ixgbe/base/ixgbe_phy.c    | 16 +++++++++---
 drivers/net/ixgbe/base/ixgbe_x540.c   | 11 ++++++--
 drivers/net/ixgbe/base/ixgbe_x550.c   | 49 +++++++++++++++++++++++++++++------
 4 files changed, 69 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_common.c b/drivers/net/ixgbe/base/ixgbe_common.c
index 0dd33b8..9645667 100644
--- a/drivers/net/ixgbe/base/ixgbe_common.c
+++ b/drivers/net/ixgbe/base/ixgbe_common.c
@@ -410,8 +410,10 @@ s32 ixgbe_start_hw_generic(struct ixgbe_hw *hw)
 
 	/* Setup flow control */
 	ret_val = ixgbe_setup_fc(hw);
-	if (ret_val != IXGBE_SUCCESS && ret_val != IXGBE_NOT_IMPLEMENTED)
+	if (ret_val != IXGBE_SUCCESS && ret_val != IXGBE_NOT_IMPLEMENTED) {
+		DEBUGOUT1("Flow control setup failed, returning %d\n", ret_val);
 		return ret_val;
+	}
 
 	/* Cache bit indicating need for crosstalk fix */
 	switch (hw->mac.type) {
@@ -501,6 +503,9 @@ s32 ixgbe_init_hw_generic(struct ixgbe_hw *hw)
 	/* Initialize the LED link active for LED blink support */
 	hw->mac.ops.init_led_link_act(hw);
 
+	if (status != IXGBE_SUCCESS)
+		DEBUGOUT1("Failed to initialize HW, STATUS = %d\n", status);
+
 	return status;
 }
 
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index e0d7125..c953805 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -291,8 +291,11 @@ static bool ixgbe_probe_phy(struct ixgbe_hw *hw, u16 phy_addr)
 {
 	u16 ext_ability = 0;
 
-	if (!ixgbe_validate_phy_addr(hw, phy_addr))
+	if (!ixgbe_validate_phy_addr(hw, phy_addr)) {
+		DEBUGOUT1("Unable to validate PHY address 0x%04X\n",
+			phy_addr);
 		return false;
+	}
 
 	if (ixgbe_get_phy_id(hw))
 		return false;
@@ -411,6 +414,8 @@ bool ixgbe_validate_phy_addr(struct ixgbe_hw *hw, u32 phy_addr)
 	if (phy_id != 0xFFFF && phy_id != 0x0)
 		valid = true;
 
+	DEBUGOUT1("PHY ID HIGH is 0x%04X\n", phy_id);
+
 	return valid;
 }
 
@@ -439,6 +444,9 @@ s32 ixgbe_get_phy_id(struct ixgbe_hw *hw)
 		hw->phy.id |= (u32)(phy_id_low & IXGBE_PHY_REVISION_MASK);
 		hw->phy.revision = (u32)(phy_id_low & ~IXGBE_PHY_REVISION_MASK);
 	}
+	DEBUGOUT2("PHY_ID_HIGH 0x%04X, PHY_ID_LOW 0x%04X\n",
+		  phy_id_high, phy_id_low);
+
 	return status;
 }
 
@@ -570,7 +578,7 @@ s32 ixgbe_reset_phy_generic(struct ixgbe_hw *hw)
  *  @phy_data: Pointer to read data from PHY register
  **/
 s32 ixgbe_read_phy_reg_mdi(struct ixgbe_hw *hw, u32 reg_addr, u32 device_type,
-		       u16 *phy_data)
+			   u16 *phy_data)
 {
 	u32 i, data, command;
 
@@ -592,12 +600,13 @@ s32 ixgbe_read_phy_reg_mdi(struct ixgbe_hw *hw, u32 reg_addr, u32 device_type,
 
 		command = IXGBE_READ_REG(hw, IXGBE_MSCA);
 		if ((command & IXGBE_MSCA_MDI_COMMAND) == 0)
-				break;
+			break;
 	}
 
 
 	if ((command & IXGBE_MSCA_MDI_COMMAND) != 0) {
 		ERROR_REPORT1(IXGBE_ERROR_POLLING, "PHY address command did not complete.\n");
+		DEBUGOUT("PHY address command did not complete, returning IXGBE_ERR_PHY\n");
 		return IXGBE_ERR_PHY;
 	}
 
@@ -627,6 +636,7 @@ s32 ixgbe_read_phy_reg_mdi(struct ixgbe_hw *hw, u32 reg_addr, u32 device_type,
 
 	if ((command & IXGBE_MSCA_MDI_COMMAND) != 0) {
 		ERROR_REPORT1(IXGBE_ERROR_POLLING, "PHY read command didn't complete\n");
+		DEBUGOUT("PHY read command didn't complete, returning IXGBE_ERR_PHY\n");
 		return IXGBE_ERR_PHY;
 	}
 
diff --git a/drivers/net/ixgbe/base/ixgbe_x540.c b/drivers/net/ixgbe/base/ixgbe_x540.c
index 49bf154..499b1fa 100644
--- a/drivers/net/ixgbe/base/ixgbe_x540.c
+++ b/drivers/net/ixgbe/base/ixgbe_x540.c
@@ -775,8 +775,10 @@ s32 ixgbe_acquire_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask)
 		/* SW NVM semaphore bit is used for access to all
 		 * SW_FW_SYNC bits (not just NVM)
 		 */
-		if (ixgbe_get_swfw_sync_semaphore(hw))
+		if (ixgbe_get_swfw_sync_semaphore(hw)) {
+			DEBUGOUT("Failed to get NVM access and register semaphore, returning IXGBE_ERR_SWFW_SYNC\n");
 			return IXGBE_ERR_SWFW_SYNC;
+		}
 
 		swfw_sync = IXGBE_READ_REG(hw, IXGBE_SWFW_SYNC_BY_MAC(hw));
 		if (!(swfw_sync & (fwmask | swmask | hwmask))) {
@@ -798,6 +800,7 @@ s32 ixgbe_acquire_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask)
 	if (swmask == IXGBE_GSSR_SW_MNG_SM) {
 		ERROR_REPORT1(IXGBE_ERROR_POLLING,
 			     "Failed to get SW only semaphore");
+		DEBUGOUT("Failed to get SW only semaphore, returning IXGBE_ERR_SWFW_SYNC\n");
 		return IXGBE_ERR_SWFW_SYNC;
 	}
 
@@ -806,8 +809,10 @@ s32 ixgbe_acquire_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask)
 	 * of the requested resource(s) while ignoring the corresponding FW/HW
 	 * bits in the SW_FW_SYNC register.
 	 */
-	if (ixgbe_get_swfw_sync_semaphore(hw))
+	if (ixgbe_get_swfw_sync_semaphore(hw)) {
+		DEBUGOUT("Failed to get NVM sempahore and register semaphore while forcefully ignoring FW sempahore bit(s) and setting SW semaphore bit(s), returning IXGBE_ERR_SWFW_SYNC\n");
 		return IXGBE_ERR_SWFW_SYNC;
+	}
 	swfw_sync = IXGBE_READ_REG(hw, IXGBE_SWFW_SYNC_BY_MAC(hw));
 	if (swfw_sync & (fwmask | hwmask)) {
 		swfw_sync |= swmask;
@@ -829,9 +834,11 @@ s32 ixgbe_acquire_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask)
 			rmask |= IXGBE_GSSR_I2C_MASK;
 		ixgbe_release_swfw_sync_X540(hw, rmask);
 		ixgbe_release_swfw_sync_semaphore(hw);
+		DEBUGOUT("Resource not released by other SW, returning IXGBE_ERR_SWFW_SYNC\n");
 		return IXGBE_ERR_SWFW_SYNC;
 	}
 	ixgbe_release_swfw_sync_semaphore(hw);
+	DEBUGOUT("Returning error IXGBE_ERR_SWFW_SYNC\n");
 
 	return IXGBE_ERR_SWFW_SYNC;
 }
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index e15054b..6f9c034 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1311,13 +1311,20 @@ s32 ixgbe_get_phy_token(struct ixgbe_hw *hw)
 					      sizeof(token_cmd),
 					      IXGBE_HI_COMMAND_TIMEOUT,
 					      true);
-	if (status)
+	if (status) {
+		DEBUGOUT1("Issuing host interface command failed with Status = %d\n",
+			  status);
 		return status;
+	}
 	if (token_cmd.hdr.cmd_or_resp.ret_status == FW_PHY_TOKEN_OK)
 		return IXGBE_SUCCESS;
-	if (token_cmd.hdr.cmd_or_resp.ret_status != FW_PHY_TOKEN_RETRY)
+	if (token_cmd.hdr.cmd_or_resp.ret_status != FW_PHY_TOKEN_RETRY) {
+		DEBUGOUT1("Host interface command returned 0x%08x , returning IXGBE_ERR_FW_RESP_INVALID\n",
+			  token_cmd.hdr.cmd_or_resp.ret_status);
 		return IXGBE_ERR_FW_RESP_INVALID;
+	}
 
+	DEBUGOUT("Returning  IXGBE_ERR_TOKEN_RETRY\n");
 	return IXGBE_ERR_TOKEN_RETRY;
 }
 
@@ -2495,9 +2502,10 @@ s32 ixgbe_reset_hw_X550em(struct ixgbe_hw *hw)
 
 	/* Call adapter stop to disable Tx/Rx and clear interrupts */
 	status = hw->mac.ops.stop_adapter(hw);
-	if (status != IXGBE_SUCCESS)
+	if (status != IXGBE_SUCCESS) {
+		DEBUGOUT1("Failed to stop adapter, STATUS = %d\n", status);
 		return status;
-
+	}
 	/* flush pending Tx transactions */
 	ixgbe_clear_tx_pending(hw);
 
@@ -2506,14 +2514,23 @@ s32 ixgbe_reset_hw_X550em(struct ixgbe_hw *hw)
 	/* PHY ops must be identified and initialized prior to reset */
 	status = hw->phy.ops.init(hw);
 
-	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED)
+	if (status)
+		DEBUGOUT1("Failed to initialize PHY ops, STATUS = %d\n",
+			  status);
+
+	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED) {
+		DEBUGOUT("Returning from reset HW since PHY ops init returned IXGBE_ERR_SFP_NOT_SUPPORTED\n");
 		return status;
+	}
 
 	/* start the external PHY */
 	if (hw->phy.type == ixgbe_phy_x550em_ext_t) {
 		status = ixgbe_init_ext_t_x550em(hw);
-		if (status)
+		if (status) {
+			DEBUGOUT1("Failed to start the external PHY, STATUS = %d\n",
+				  status);
 			return status;
+		}
 	}
 
 	/* Setup SFP module if there is one present. */
@@ -2587,6 +2604,9 @@ s32 ixgbe_reset_hw_X550em(struct ixgbe_hw *hw)
 	if (hw->device_id == IXGBE_DEV_ID_X550EM_X_SFP)
 		ixgbe_setup_mux_ctl(hw);
 
+	if (status != IXGBE_SUCCESS)
+		DEBUGOUT1("Reset HW failed, STATUS = %d\n", status);
+
 	return status;
 }
 
@@ -4336,21 +4356,34 @@ STATIC s32 ixgbe_acquire_swfw_sync_X550a(struct ixgbe_hw *hw, u32 mask)
 		status = IXGBE_SUCCESS;
 		if (hmask)
 			status = ixgbe_acquire_swfw_sync_X540(hw, hmask);
-		if (status)
+		if (status) {
+			DEBUGOUT1("Could not acquire SWFW semaphore, Status = %d\n",
+				  status);
 			return status;
+		}
 		if (!(mask & IXGBE_GSSR_TOKEN_SM))
 			return IXGBE_SUCCESS;
 
 		status = ixgbe_get_phy_token(hw);
+		if (status == IXGBE_ERR_TOKEN_RETRY)
+			DEBUGOUT1("Could not acquire PHY token, Status = %d\n",
+				  status);
+
 		if (status == IXGBE_SUCCESS)
 			return IXGBE_SUCCESS;
 
 		if (hmask)
 			ixgbe_release_swfw_sync_X540(hw, hmask);
-		if (status != IXGBE_ERR_TOKEN_RETRY)
+
+		if (status != IXGBE_ERR_TOKEN_RETRY) {
+			DEBUGOUT1("Unable to retry acquiring the PHY token, Status = %d\n",
+				  status);
 			return status;
+		}
 	}
 
+	DEBUGOUT1("Semaphore acquisition retries failed!: PHY ID = 0x%08X\n",
+		  hw->phy.id);
 	return status;
 }
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH 6/7] net/ixgbe/base: add some debug traces

[Ferruh Yigit]

On 1/10/2017 3:45 PM, Wei Dai wrote:
> This patch adds some traces in the reset_hw logic
> and semaphore acquisition logic to help debugging.

Is verbosity of the PMD changes with this update?

Is something required in PMD to disable DEBUGOUT1(), DEBUGOUT2(),
DEBUGOUT() by default?

> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> ---

<...>

Re: [dpdk-dev] [PATCH 6/7] net/ixgbe/base: add some debug traces

[Dai, Wei]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, January 11, 2017 11:39 PM
> To: Dai, Wei <wei.dai@intel.com>; dev@dpdk.org
> Cc: Zhang, Helin <helin.zhang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 6/7] net/ixgbe/base: add some debug traces
> 
> On 1/10/2017 3:45 PM, Wei Dai wrote:
> > This patch adds some traces in the reset_hw logic and semaphore
> > acquisition logic to help debugging.
> 
> Is verbosity of the PMD changes with this update?
> 
> Is something required in PMD to disable DEBUGOUT1(), DEBUGOUT2(),
> DEBUGOUT() by default?

In drivers/net/ixgbe/ixgbe_logs.h, there are
#ifdef RTE_LIBRTE_IXGBE_DEBUG_DRIVER
#define PMD_DRV_LOG_RAW(level, fmt, args...) \
	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
#else
#define PMD_DRV_LOG_RAW(level, fmt, args...) do { } while (0)
#endif

And in drivers/net/ixgbe/base/ixgbe_osdep.h, there are
#include "../ixgbe_logs.h"
....
#define DEBUGOUT(S, args...)    PMD_DRV_LOG_RAW(DEBUG, S, ##args)
#define DEBUGOUT1(S, args...)   DEBUGOUT(S, ##args)
#define DEBUGOUT2(S, args...)   DEBUGOUT(S, ##args)
#define DEBUGOUT3(S, args...)   DEBUGOUT(S, ##args)
#define DEBUGOUT6(S, args...)   DEBUGOUT(S, ##args)
#define DEBUGOUT7(S, args...)   DEBUGOUT(S, ##args)

In config/common_base, there is 
CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n

So according above codes, the macro RTE_LIBRTE_IXGBE_DEBUG_DRIVER in drivers/net/ixgbe/ixgbe_logs.h is not defined
and PMD_DRV_LOG_RAW(level, fmt, args...) do { } while (0) will be valid,
then DEBUGOUT() is just a null definition by default.

> 
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > ---
> 
> <...>

Re: [dpdk-dev] [PATCH 6/7] net/ixgbe/base: add some debug traces

[Ferruh Yigit]

On 1/12/2017 2:18 PM, Dai, Wei wrote:
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Wednesday, January 11, 2017 11:39 PM
>> To: Dai, Wei <wei.dai@intel.com>; dev@dpdk.org
>> Cc: Zhang, Helin <helin.zhang@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH 6/7] net/ixgbe/base: add some debug traces
>>
>> On 1/10/2017 3:45 PM, Wei Dai wrote:
>>> This patch adds some traces in the reset_hw logic and semaphore
>>> acquisition logic to help debugging.
>>
>> Is verbosity of the PMD changes with this update?
>>
>> Is something required in PMD to disable DEBUGOUT1(), DEBUGOUT2(),
>> DEBUGOUT() by default?
> 
> In drivers/net/ixgbe/ixgbe_logs.h, there are
> #ifdef RTE_LIBRTE_IXGBE_DEBUG_DRIVER
> #define PMD_DRV_LOG_RAW(level, fmt, args...) \
> 	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
> #else
> #define PMD_DRV_LOG_RAW(level, fmt, args...) do { } while (0)
> #endif
> 
> And in drivers/net/ixgbe/base/ixgbe_osdep.h, there are
> #include "../ixgbe_logs.h"
> ....
> #define DEBUGOUT(S, args...)    PMD_DRV_LOG_RAW(DEBUG, S, ##args)
> #define DEBUGOUT1(S, args...)   DEBUGOUT(S, ##args)
> #define DEBUGOUT2(S, args...)   DEBUGOUT(S, ##args)
> #define DEBUGOUT3(S, args...)   DEBUGOUT(S, ##args)
> #define DEBUGOUT6(S, args...)   DEBUGOUT(S, ##args)
> #define DEBUGOUT7(S, args...)   DEBUGOUT(S, ##args)
> 
> In config/common_base, there is 
> CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n
> 
> So according above codes, the macro RTE_LIBRTE_IXGBE_DEBUG_DRIVER in drivers/net/ixgbe/ixgbe_logs.h is not defined
> and PMD_DRV_LOG_RAW(level, fmt, args...) do { } while (0) will be valid,
> then DEBUGOUT() is just a null definition by default.

OK, thanks for clarification.

> 
>>
>>>
>>> Signed-off-by: Wei Dai <wei.dai@intel.com>
>>> ---
>>
>> <...>

[dpdk-dev] [PATCH 7/7] net/ixgbe/base: update version of shared code

[Wei Dai]

update version of shared code to 2017-01-05 in README.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/base/README b/drivers/net/ixgbe/base/README
index 1936191..0a6054f 100644
--- a/drivers/net/ixgbe/base/README
+++ b/drivers/net/ixgbe/base/README
@@ -34,7 +34,7 @@ Intel® IXGBE driver
 ===================
 
 This directory contains source code of FreeBSD ixgbe driver of version
-cid-10g-shared-code.2016.11.21 released by the team which develop
+cid-10g-shared-code.2017.01.05 released by the team which develop
 basic drivers for any ixgbe NIC. The sub-directory of base/
 contains the original source package.
 This driver is valid for the product(s) listed below
-- 
2.7.4