[dpdk-dev] net/e1000: add minimum support for Broadcom 54616 PHY

Message ID 20171206235528.29746-1-3chas3@gmail.com (mailing list archive)
State Accepted, archived
Delegated to: Helin Zhang
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Chas Williams Dec. 6, 2017, 11:55 p.m. UTC
  From: "Charles (Chas) Williams" <ciwillia@brocade.com>

If we find a Broadcom 54616, handle as a e1000_phy_none assuming that
the NIC reset has initialized the PHY to a sane state.

Signed-off-by: Chas Williams <chas3@att.com>
---
 drivers/net/e1000/base/e1000_82575.c   | 5 +++++
 drivers/net/e1000/base/e1000_defines.h | 1 +
 2 files changed, 6 insertions(+)
  

Comments

Chas Williams Dec. 7, 2017, midnight UTC | #1
Sorry, sent this to the wrong maintainer initially.  I am cc'ing the
Broadcom bnxt maintainers in hopes that they might comment about the 54616
behavior on reset.  I couldn't find any programmer guide for this chip
online but PHYs are typically well behaved.  This patch does work on our
test system.

On Wed, Dec 6, 2017 at 6:55 PM, Chas Williams <3chas3@gmail.com> wrote:

> From: "Charles (Chas) Williams" <ciwillia@brocade.com>
>
> If we find a Broadcom 54616, handle as a e1000_phy_none assuming that
> the NIC reset has initialized the PHY to a sane state.
>
> Signed-off-by: Chas Williams <chas3@att.com>
> ---
>  drivers/net/e1000/base/e1000_82575.c   | 5 +++++
>  drivers/net/e1000/base/e1000_defines.h | 1 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/net/e1000/base/e1000_82575.c
> b/drivers/net/e1000/base/e1000_82575.c
> index c6400bd..3bcb6da 100644
> --- a/drivers/net/e1000/base/e1000_82575.c
> +++ b/drivers/net/e1000/base/e1000_82575.c
> @@ -312,6 +312,9 @@ STATIC s32 e1000_init_phy_params_82575(struct
> e1000_hw *hw)
>                 phy->ops.set_d3_lplu_state = e1000_set_d3_lplu_state_82580;
>                 phy->ops.force_speed_duplex = e1000_phy_force_speed_duplex_
> m88;
>                 break;
> +       case BCM54616_E_PHY_ID:
> +               phy->type               = e1000_phy_none;
> +               break;
>         default:
>                 ret_val = -E1000_ERR_PHY;
>                 goto out;
> @@ -1607,6 +1610,8 @@ STATIC s32 e1000_setup_copper_link_82575(struct
> e1000_hw *hw)
>         case e1000_phy_82580:
>                 ret_val = e1000_copper_link_setup_82577(hw);
>                 break;
> +       case e1000_phy_none:
> +               break;
>         default:
>                 ret_val = -E1000_ERR_PHY;
>                 break;
> diff --git a/drivers/net/e1000/base/e1000_defines.h
> b/drivers/net/e1000/base/e1000_defines.h
> index dbc2bbb..e2101c1 100644
> --- a/drivers/net/e1000/base/e1000_defines.h
> +++ b/drivers/net/e1000/base/e1000_defines.h
> @@ -1274,6 +1274,7 @@ POSSIBILITY OF SUCH DAMAGE.
>  #define I350_I_PHY_ID          0x015403B0
>  #define I210_I_PHY_ID          0x01410C00
>  #define IGP04E1000_E_PHY_ID    0x02A80391
> +#define BCM54616_E_PHY_ID      0x03625D10
>  #define M88_VENDOR             0x0141
>
>  /* M88E1000 Specific Registers */
> --
> 2.9.5
>
>
  
Wenzhuo Lu Jan. 11, 2018, 8:30 a.m. UTC | #2
Hi Chas,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Chas Williams
> Sent: Thursday, December 7, 2017 7:55 AM
> To: dev@dpdk.org
> Cc: skhare@vmware.com; Charles (Chas) Williams <ciwillia@brocade.com>;
> Chas Williams <chas3@att.com>
> Subject: [dpdk-dev] [PATCH] net/e1000: add minimum support for Broadcom
> 54616 PHY
> 
> From: "Charles (Chas) Williams" <ciwillia@brocade.com>
> 
> If we find a Broadcom 54616, handle as a e1000_phy_none assuming that
> the NIC reset has initialized the PHY to a sane state.
> 
> Signed-off-by: Chas Williams <chas3@att.com>

> b/drivers/net/e1000/base/e1000_defines.h
> index dbc2bbb..e2101c1 100644
> --- a/drivers/net/e1000/base/e1000_defines.h
> +++ b/drivers/net/e1000/base/e1000_defines.h
> @@ -1274,6 +1274,7 @@ POSSIBILITY OF SUCH DAMAGE.
>  #define I350_I_PHY_ID		0x015403B0
>  #define I210_I_PHY_ID		0x01410C00
>  #define IGP04E1000_E_PHY_ID	0x02A80391
> +#define BCM54616_E_PHY_ID	0x03625D10
TBH, normally we don't change the base code. I checked the kernel driver and don't find this PHY either. May I ask if it's an Intel NIC and what's the device ID? Thanks.
  
Chas Williams Jan. 11, 2018, 5:57 p.m. UTC | #3
I derived that value by observing what the ixgbe finds when loading on my
platform.  The docs for the platform say that the attached PHY is a
Broadcom 54616.  I couldn't find any documentation on this PHY to determine
what this value is (i.e. is it really some sort of ID string for that
PHY).    I almost suspect that the ixgbe is reading some registers on the
PHY that just happen to contain this value after reset.

Supposedly it's similar to the 54618 but that really doesn't provide much
more information.  The bnx2x has a driver for the 54618 and nothing that
looks similar to that string.
  
Wenzhuo Lu Jan. 15, 2018, 1:11 a.m. UTC | #4
Hi Chas,
1, Would you like to use this CLI “lspci -nn | grep Eth” to get the device ID of  the NIC?
2, You mentioned “the doc of the platform”. Is it public? If so, could you tell me the link? Just want to understand more about it.
Thanks.


Best regards
Wenzhuo Lu

From: Chas Williams [mailto:3chas3@gmail.com]

Sent: Friday, January 12, 2018 1:57 AM
To: Lu, Wenzhuo <wenzhuo.lu@intel.com>
Cc: dev@dpdk.org; skhare@vmware.com; Charles (Chas) Williams <ciwillia@brocade.com>; Chas Williams <chas3@att.com>
Subject: Re: [dpdk-dev] [PATCH] net/e1000: add minimum support for Broadcom 54616 PHY

I derived that value by observing what the ixgbe finds when loading on my platform.  The docs for the platform say that the attached PHY is a Broadcom 54616.  I couldn't find any documentation on this PHY to determine what this value is (i.e. is it really some sort of ID string for that PHY).    I almost suspect that the ixgbe is reading some registers on the PHY that just happen to contain this value after reset.

Supposedly it's similar to the 54618 but that really doesn't provide much more information.  The bnx2x has a driver for the 54618 and nothing that looks similar to that string.
  
Chas Williams Jan. 15, 2018, 1:01 p.m. UTC | #5
00:14.0 Ethernet controller [0200]: Intel Corporation Ethernet Connection
I354 [8086:1f41] (rev 03)

I am not clear on the status of the document I have.  Here is a link to a
similar product: https://people.ucsc.edu/~warner/Bufs/AG6248.pdf -- I have
the 7648c which is somewhat similar to the 6248c as far as the "out of
band" ports.

On Sun, Jan 14, 2018 at 8:11 PM, Lu, Wenzhuo <wenzhuo.lu@intel.com> wrote:

> Hi Chas,
>
> 1, Would you like to use this CLI “lspci -nn | grep Eth” to get the device
> ID of  the NIC?
>
> 2, You mentioned “the doc of the platform”. Is it public? If so, could you
> tell me the link? Just want to understand more about it.
>
> Thanks.
>
>
>
>
>
> Best regards
>
> Wenzhuo Lu
>
>
>
> *From:* Chas Williams [mailto:3chas3@gmail.com]
> *Sent:* Friday, January 12, 2018 1:57 AM
> *To:* Lu, Wenzhuo <wenzhuo.lu@intel.com>
> *Cc:* dev@dpdk.org; skhare@vmware.com; Charles (Chas) Williams <
> ciwillia@brocade.com>; Chas Williams <chas3@att.com>
> *Subject:* Re: [dpdk-dev] [PATCH] net/e1000: add minimum support for
> Broadcom 54616 PHY
>
>
>
> I derived that value by observing what the ixgbe finds when loading on my
> platform.  The docs for the platform say that the attached PHY is a
> Broadcom 54616.  I couldn't find any documentation on this PHY to determine
> what this value is (i.e. is it really some sort of ID string for that
> PHY).    I almost suspect that the ixgbe is reading some registers on the
> PHY that just happen to contain this value after reset.
>
>
>
> Supposedly it's similar to the 54618 but that really doesn't provide much
> more information.  The bnx2x has a driver for the 54618 and nothing that
> looks similar to that string.
>
  
Wenzhuo Lu Jan. 18, 2018, 12:39 a.m. UTC | #6
Hi Chas,

Thanks for your info and the patch. I think we should have it.



Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com<mailto:wenzhuo.lu@intel.com>>



Best regards
Wenzhuo Lu

From: Chas Williams [mailto:3chas3@gmail.com]

Sent: Friday, January 12, 2018 1:57 AM
To: Lu, Wenzhuo <wenzhuo.lu@intel.com>
Cc: dev@dpdk.org; skhare@vmware.com; Charles (Chas) Williams <ciwillia@brocade.com>; Chas Williams <chas3@att.com>
Subject: Re: [dpdk-dev] [PATCH] net/e1000: add minimum support for Broadcom 54616 PHY

I derived that value by observing what the ixgbe finds when loading on my platform.  The docs for the platform say that the attached PHY is a Broadcom 54616.  I couldn't find any documentation on this PHY to determine what this value is (i.e. is it really some sort of ID string for that PHY).    I almost suspect that the ixgbe is reading some registers on the PHY that just happen to contain this value after reset.

Supposedly it's similar to the 54618 but that really doesn't provide much more information.  The bnx2x has a driver for the 54618 and nothing that looks similar to that string.
  
Zhang, Helin May 8, 2018, 1:48 a.m. UTC | #7
> -----Original Message-----

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Lu, Wenzhuo

> Sent: Thursday, January 18, 2018 8:40 AM

> To: Chas Williams

> Cc: dev@dpdk.org; skhare@vmware.com; Charles (Chas) Williams; Chas

> Williams

> Subject: Re: [dpdk-dev] [PATCH] net/e1000: add minimum support for

> Broadcom 54616 PHY

> 

> Hi Chas,

> 

> Thanks for your info and the patch. I think we should have it.

> 

> 

> 

> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Applied to dpdk-next-net-intel, thanks!

/Helin
  

Patch

diff --git a/drivers/net/e1000/base/e1000_82575.c b/drivers/net/e1000/base/e1000_82575.c
index c6400bd..3bcb6da 100644
--- a/drivers/net/e1000/base/e1000_82575.c
+++ b/drivers/net/e1000/base/e1000_82575.c
@@ -312,6 +312,9 @@  STATIC s32 e1000_init_phy_params_82575(struct e1000_hw *hw)
 		phy->ops.set_d3_lplu_state = e1000_set_d3_lplu_state_82580;
 		phy->ops.force_speed_duplex = e1000_phy_force_speed_duplex_m88;
 		break;
+	case BCM54616_E_PHY_ID:
+		phy->type		= e1000_phy_none;
+		break;
 	default:
 		ret_val = -E1000_ERR_PHY;
 		goto out;
@@ -1607,6 +1610,8 @@  STATIC s32 e1000_setup_copper_link_82575(struct e1000_hw *hw)
 	case e1000_phy_82580:
 		ret_val = e1000_copper_link_setup_82577(hw);
 		break;
+	case e1000_phy_none:
+		break;
 	default:
 		ret_val = -E1000_ERR_PHY;
 		break;
diff --git a/drivers/net/e1000/base/e1000_defines.h b/drivers/net/e1000/base/e1000_defines.h
index dbc2bbb..e2101c1 100644
--- a/drivers/net/e1000/base/e1000_defines.h
+++ b/drivers/net/e1000/base/e1000_defines.h
@@ -1274,6 +1274,7 @@  POSSIBILITY OF SUCH DAMAGE.
 #define I350_I_PHY_ID		0x015403B0
 #define I210_I_PHY_ID		0x01410C00
 #define IGP04E1000_E_PHY_ID	0x02A80391
+#define BCM54616_E_PHY_ID	0x03625D10
 #define M88_VENDOR		0x0141
 
 /* M88E1000 Specific Registers */