From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A9A1CA0547; Wed, 12 Oct 2022 10:39:44 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 99B4842D6E; Wed, 12 Oct 2022 10:39:44 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 493AE42B6D for ; Wed, 12 Oct 2022 10:39:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1665563982; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vUUkeCzCG75libOx+HFHmAeJvwMWRSEVDuv1Re/lK1A=; b=Pj6ZYgtzZgjBxBYyFthSa7daJ0r2LP9FXhINjT9xUYN7N2UiXHTHbv3XqviMdcN1Rtqoc0 1mq1ElMy1JASGffRygNbQJvarbMjDlFDwMeyVTeeSw4OXSpD+rukEPO1ABvK65223oYFQz RrQzRLtqZqYBtw/nCF0Ge1ANJTr09nM= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-145-RGNt3YsENom_NKfwv8ry3w-1; Wed, 12 Oct 2022 04:39:41 -0400 X-MC-Unique: RGNt3YsENom_NKfwv8ry3w-1 Received: by mail-wm1-f71.google.com with SMTP id d5-20020a05600c34c500b003b4fb42ccdeso794945wmq.8 for ; Wed, 12 Oct 2022 01:39:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:subject:from:content-language :references:cc:to:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=vUUkeCzCG75libOx+HFHmAeJvwMWRSEVDuv1Re/lK1A=; b=xcafDsjS5rDGkNSoQu0lZc+bAODT3W0JYtweKPU+EbcCCEFprUgKU7vADlsX2mO14K 6TzVmKqCh9wJpUCThByR1Ux05agOdXfKNdIAjmTaO9XI+if92WeGIIIMH4k3nIRgzEAK dqLKViidzAM/dKBxB8vMzEdMAoj/1a8w1LeisoDrzXNO4TakQbr1alb84YNKjgmsekY+ S4tUA7g998J3idsI21UHnGxL0RMNRbCP5NwaRRa1huhDQ7kRysJ77w8jysaoclnwKlW+ OJykhpAEip/KPW1AujP3T7I+vLfQh/4jNRv8paH0HM1t5QIItl+qAh/jCLROan0XgUft FhDg== X-Gm-Message-State: ACrzQf3Ld5SxYEDDkbekVPTJX5PIuZmvybiEDeE/fQVEk3CRp1UJVBxo AccVLQD8CGGJ4510f8V4OIeVEfzr9VtVj4EfaGyR/vUxrZXKfl8IeBFuvgFctDESQI6D/qVSMDs 1mz4= X-Received: by 2002:adf:db42:0:b0:22e:386e:f6af with SMTP id f2-20020adfdb42000000b0022e386ef6afmr16918864wrj.400.1665563980586; Wed, 12 Oct 2022 01:39:40 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7f8nD83/2JnuVCs5WI4FCs2j4IXQ3MwXgCwD26+lIVXFgCJGNrV0616S9yhhx5wEY6AcRB8g== X-Received: by 2002:adf:db42:0:b0:22e:386e:f6af with SMTP id f2-20020adfdb42000000b0022e386ef6afmr16918854wrj.400.1665563980408; Wed, 12 Oct 2022 01:39:40 -0700 (PDT) Received: from [192.168.0.36] ([78.19.110.164]) by smtp.gmail.com with ESMTPSA id ba6-20020a0560001c0600b0022e57e66824sm16718381wrb.99.2022.10.12.01.39.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 12 Oct 2022 01:39:40 -0700 (PDT) Message-ID: <31c280bd-2d69-e302-8ec6-cbff21badd4a@redhat.com> Date: Wed, 12 Oct 2022 09:39:38 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 To: "Mah, Yock Gen" , IOTG DPDK Ref App , "Zhang, Qi Z" Cc: "dev@dpdk.org" References: <20220829081436.29945-1-iotg.dpdk.ref.app@intel.com> <20220831225139.78201-1-iotg.dpdk.ref.app@intel.com> <45165da1-25af-b735-f80d-a6b42592cf6c@redhat.com> From: Kevin Traynor Subject: Re: [PATCH v2] IGC: Remove I225_I_PHY_ID checking In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 12/10/2022 08:45, Mah, Yock Gen wrote: > > > -----Original Message----- > From: Kevin Traynor > Sent: Tuesday, 11 October, 2022 7:05 PM > To: IOTG DPDK Ref App ; Zhang, Qi Z > Cc: dev@dpdk.org > Subject: Re: [PATCH v2] IGC: Remove I225_I_PHY_ID checking > > On 31/08/2022 23:51, iotg.dpdk.ref.app@intel.com wrote: >> From: NSWE SWS DPDK Dev >> >> 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 >> --- >> drivers/net/igc/base/igc_i225.c | 15 ++------------- >> drivers/net/igc/base/igc_phy.c | 6 ++---- >> 2 files changed, 4 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/net/igc/base/igc_i225.c >> b/drivers/net/igc/base/igc_i225.c index 5f3d535490..af26602afb 100644 >> --- a/drivers/net/igc/base/igc_i225.c >> +++ b/drivers/net/igc/base/igc_i225.c >> @@ -173,19 +173,8 @@ static s32 igc_init_phy_params_i225(struct igc_hw *hw) >> phy->ops.write_reg = igc_write_phy_reg_gpy; >> >> ret_val = igc_get_phy_id(hw); >> - /* Verify phy id and set remaining function pointers */ >> - switch (phy->id) { >> - case I225_I_PHY_ID: >> - case I226_LM_PHY_ID: >> - phy->type = igc_phy_i225; >> - phy->ops.set_d0_lplu_state = igc_set_d0_lplu_state_i225; >> - phy->ops.set_d3_lplu_state = igc_set_d3_lplu_state_i225; > >> - The commit log says it is removing a check on the ID, but it does not say why these function pointers are being removed. > >> - Why are they removed, were they not needed? > > > i225 devices have only one PHY vendor. There is no point checking _I_PHY_ID during the link establishment and auto-negotiation process. > Right, that's clear about the vendor ID check. But it's not clear to me why the the resulting code like this: phy->type = igc_phy_i225; and not like this: phy->type = igc_phy_i225; phy->ops.set_d0_lplu_state = igc_set_d0_lplu_state_i225; phy->ops.set_d3_lplu_state = igc_set_d3_lplu_state_i225; So it is using dummy null functions instead: https://git.dpdk.org/dpdk/tree/drivers/net/igc/base/igc_phy.c#n61 Do the device registers not need to be set anymore? >> - /* TODO - complete with GPY PHY information */ >> - break; >> - default: >> - ret_val = -IGC_ERR_PHY; >> - goto out; >> - } >> + phy->type = igc_phy_i225; >> + >> >> out: >> return ret_val; >> diff --git a/drivers/net/igc/base/igc_phy.c >> b/drivers/net/igc/base/igc_phy.c index 43bbe69bca..2906bae21a 100644 >> --- a/drivers/net/igc/base/igc_phy.c >> +++ b/drivers/net/igc/base/igc_phy.c >> @@ -1474,8 +1474,7 @@ s32 igc_phy_setup_autoneg(struct igc_hw *hw) >> return ret_val; >> } >> >> - if ((phy->autoneg_mask & ADVERTISE_2500_FULL) && >> - hw->phy.id == I225_I_PHY_ID) { >> + if (phy->autoneg_mask & ADVERTISE_2500_FULL) { >> /* Read the MULTI GBT AN Control Register - reg 7.32 */ >> ret_val = phy->ops.read_reg(hw, (STANDARD_AN_REG_MASK << >> MMD_DEVADDR_SHIFT) | >> @@ -1615,8 +1614,7 @@ s32 igc_phy_setup_autoneg(struct igc_hw *hw) >> ret_val = phy->ops.write_reg(hw, PHY_1000T_CTRL, >> mii_1000t_ctrl_reg); >> >> - if ((phy->autoneg_mask & ADVERTISE_2500_FULL) && >> - hw->phy.id == I225_I_PHY_ID) >> + if (phy->autoneg_mask & ADVERTISE_2500_FULL) >> ret_val = phy->ops.write_reg(hw, >> (STANDARD_AN_REG_MASK << >> MMD_DEVADDR_SHIFT) | >