IGC: Remove I225_I_PHY_ID checking

  From: NSWE SWS DPDK Dev <iotg.dpdk.ref.app@intel.com>

i225 devices have only one PHY vendor. There is unnecessary to check _I_PHY_ID during the link establishment
and auto-negotiation process, the checking also caused devices like i225-IT failed. This patch is to remove the
mentioned unnecessary checking.

Cc: stable@dpdk.org
Signed-off-by: NSWE SWS DPDK Dev <iotg.dpdk.ref.app@intel.com>
---
 drivers/net/igc/base/igc_api.c  |  1 +
 drivers/net/igc/base/igc_hw.h   |  1 +
 drivers/net/igc/base/igc_i225.c | 15 ++-------------
 drivers/net/igc/base/igc_phy.c  |  6 ++----
 drivers/net/igc/igc_ethdev.c    |  1 +
 5 files changed, 7 insertions(+), 17 deletions(-)
  

> -----Original Message-----
> From: iotg.dpdk.ref.app@intel.com <iotg.dpdk.ref.app@intel.com>
> Sent: Monday, August 29, 2022 4:15 PM
> To: dev@dpdk.org
> Subject: [PATCH] IGC: Remove I225_I_PHY_ID checking
> 
> From: NSWE SWS DPDK Dev <iotg.dpdk.ref.app@intel.com>
> 
> i225 devices have only one PHY vendor. There is unnecessary to check
> _I_PHY_ID during the link establishment and auto-negotiation process, the
> checking also caused devices like i225-IT failed. This patch is to remove the
> mentioned unnecessary checking.
> 
> Cc: stable@dpdk.org
> Signed-off-by: NSWE SWS DPDK Dev <iotg.dpdk.ref.app@intel.com>

Is this the expected author name?

> ---
>  drivers/net/igc/base/igc_api.c  |  1 +
>  drivers/net/igc/base/igc_hw.h   |  1 +
>  drivers/net/igc/base/igc_i225.c | 15 ++-------------
> drivers/net/igc/base/igc_phy.c  |  6 ++----
>  drivers/net/igc/igc_ethdev.c    |  1 +
>  5 files changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/igc/base/igc_api.c b/drivers/net/igc/base/igc_api.c
> index 9b791dc082..c9fc9ed4b0 100644
> --- a/drivers/net/igc/base/igc_api.c
> +++ b/drivers/net/igc/base/igc_api.c
> @@ -886,6 +886,7 @@ s32 igc_set_mac_type(struct igc_hw *hw)
>  	case IGC_DEV_ID_I225_V:
>  	case IGC_DEV_ID_I225_K:
>  	case IGC_DEV_ID_I225_I:
> +	case IGC_DEV_ID_I225_IT:
>  	case IGC_DEV_ID_I220_V:
>  	case IGC_DEV_ID_I225_BLANK_NVM:
>  	case IGC_DEV_ID_I226_K:
> diff --git a/drivers/net/igc/base/igc_hw.h b/drivers/net/igc/base/igc_hw.h
> index 707a1883b4..e919a11c02 100644
> --- a/drivers/net/igc/base/igc_hw.h
> +++ b/drivers/net/igc/base/igc_hw.h
> @@ -164,6 +164,7 @@ struct igc_hw;
>  #define IGC_DEV_ID_I225_V			0x15F3
>  #define IGC_DEV_ID_I225_K			0x3100
>  #define IGC_DEV_ID_I225_I			0x15F8
> +#define IGC_DEV_ID_I225_IT			0x0D9F

The patch's commit log claims to remove something, 
but in code it also add some new device ID, could you clarify why we need this or it should be in a separate patch?
  

>-----Original Message-----
>From: Zhang, Qi Z <qi.z.zhang@intel.com> 
>Sent: Tuesday, 30 August, 2022 7:17 PM
>To: IOTG DPDK Ref App <iotg.dpdk.ref.app@intel.com>; dev@dpdk.org
>Subject: RE: [PATCH] IGC: Remove I225_I_PHY_ID checking

> -----Original Message-----
> From: iotg.dpdk.ref.app@intel.com <iotg.dpdk.ref.app@intel.com>
> Sent: Monday, August 29, 2022 4:15 PM
> To: dev@dpdk.org
> Subject: [PATCH] IGC: Remove I225_I_PHY_ID checking
> 
> From: NSWE SWS DPDK Dev <iotg.dpdk.ref.app@intel.com>
> 
> i225 devices have only one PHY vendor. There is unnecessary to check 
> _I_PHY_ID during the link establishment and auto-negotiation process, 
> the checking also caused devices like i225-IT failed. This patch is to 
> remove the mentioned unnecessary checking.
> 
> Cc: stable@dpdk.org
> Signed-off-by: NSWE SWS DPDK Dev <iotg.dpdk.ref.app@intel.com>

>Is this the expected author name?
Yes, this is expected author name, the PDL email contain all developers working on the patch for better support and response. 
> ---
>  drivers/net/igc/base/igc_api.c  |  1 +
>  drivers/net/igc/base/igc_hw.h   |  1 +
>  drivers/net/igc/base/igc_i225.c | 15 ++------------- 
> drivers/net/igc/base/igc_phy.c  |  6 ++----
>  drivers/net/igc/igc_ethdev.c    |  1 +
>  5 files changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/igc/base/igc_api.c 
> b/drivers/net/igc/base/igc_api.c index 9b791dc082..c9fc9ed4b0 100644
> --- a/drivers/net/igc/base/igc_api.c
> +++ b/drivers/net/igc/base/igc_api.c
> @@ -886,6 +886,7 @@ s32 igc_set_mac_type(struct igc_hw *hw)
>  	case IGC_DEV_ID_I225_V:
>  	case IGC_DEV_ID_I225_K:
>  	case IGC_DEV_ID_I225_I:
> +	case IGC_DEV_ID_I225_IT:
>  	case IGC_DEV_ID_I220_V:
>  	case IGC_DEV_ID_I225_BLANK_NVM:
>  	case IGC_DEV_ID_I226_K:
> diff --git a/drivers/net/igc/base/igc_hw.h 
> b/drivers/net/igc/base/igc_hw.h index 707a1883b4..e919a11c02 100644
> --- a/drivers/net/igc/base/igc_hw.h
> +++ b/drivers/net/igc/base/igc_hw.h
> @@ -164,6 +164,7 @@ struct igc_hw;
>  #define IGC_DEV_ID_I225_V			0x15F3
>  #define IGC_DEV_ID_I225_K			0x3100
>  #define IGC_DEV_ID_I225_I			0x15F8
> +#define IGC_DEV_ID_I225_IT			0x0D9F

>The patch's commit log claims to remove something, but in code it also add some new device ID, could you clarify why we need this or it should be in a separate patch?
The checking issue was firstly discovered because of i225-IT not working on our lab and escalated to DPDK team for support, as informed by DPDK team after the patch sent, that the i225-IT device ID has been patched in DPDK next release, we'll send out v2 patch soon with only removing redundance checking without i225-IT device ID.
  

On Thu, Sep 1, 2022 at 10:08 AM Mah, Yock Gen <yock.gen.mah@intel.com> wrote:
>
> >-----Original Message-----
> >From: Zhang, Qi Z <qi.z.zhang@intel.com>
> >Sent: Tuesday, 30 August, 2022 7:17 PM
> >To: IOTG DPDK Ref App <iotg.dpdk.ref.app@intel.com>; dev@dpdk.org
> >Subject: RE: [PATCH] IGC: Remove I225_I_PHY_ID checking
>
> > -----Original Message-----
> > From: iotg.dpdk.ref.app@intel.com <iotg.dpdk.ref.app@intel.com>
> > Sent: Monday, August 29, 2022 4:15 PM
> > To: dev@dpdk.org
> > Subject: [PATCH] IGC: Remove I225_I_PHY_ID checking
> >
> > From: NSWE SWS DPDK Dev <iotg.dpdk.ref.app@intel.com>
> >
> > i225 devices have only one PHY vendor. There is unnecessary to check
> > _I_PHY_ID during the link establishment and auto-negotiation process,
> > the checking also caused devices like i225-IT failed. This patch is to
> > remove the mentioned unnecessary checking.
> >
> > Cc: stable@dpdk.org
> > Signed-off-by: NSWE SWS DPDK Dev <iotg.dpdk.ref.app@intel.com>
>
> >Is this the expected author name?
> Yes, this is expected author name, the PDL email contain all developers working on the patch for better support and response.

This is not acceptable.

https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-body

It is clearly stated that:
"""
The signoff must be a real name and not an alias or nickname. More
than one signoff is allowed.
"""

And all developers involved in the making of a patch must follow the
Developer’s Certificate of Origin.
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1

In its current form, this patch cannot be merged in the DPDK project.


Besides, please register to the @dev mailing list.
  

On Thu, Sep 1, 2022 at 10:22 AM David Marchand
<david.marchand@redhat.com> wrote:
> Besides, please register to the @dev mailing list.

Bis.

Please register to the @dev mailing list.