Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e71e44d5-356c-7689-4d4d-d95049c76c3d@intel.com>
Date:   Sun, 4 Sep 2022 08:42:40 +0300
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.2.1
Subject: Re: [Intel-wired-lan] [PATCH v3] drivers/net/ethernet/e1000e: check
 return value of e1e_rphy()
To:     Li Zhong <floridsleeves@gmail.com>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <intel-wired-lan@lists.osuosl.org>
CC:     <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <davem@davemloft.net>, "Neftin, Sasha" <sasha.neftin@intel.com>
References: <20220830071549.2137413-1-floridsleeves@gmail.com>
Content-Language: en-US
From:   "Neftin, Sasha" <sasha.neftin@intel.com>
In-Reply-To: <20220830071549.2137413-1-floridsleeves@gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?T1NaalJsa1c5KzVFYnlZSjBPSnRhZllrTWt4cW93OTByTWhpU25OaEpiQ0Vy?=
 =?utf-8?B?cjcxTFI0d2x4d2xRUE1TZjNBQkdMeTRTUkdDaW5pWjFZL3JNZGhhUEswSEU2?=
 =?utf-8?B?bVNlTURxaFRJTXN3VlFoUjQ4OWYyUEpHSzZPTmJWUGM2R2c1ekcwYmsyYlJG?=
 =?utf-8?B?YldmS01jRXZLWngzOGVRSG5KbEdtLzNNMUJ1SG80SSs5MzMzRnNReVVmKzFX?=
 =?utf-8?B?dTExWWI5c0RzczAzNjRkOHRhQ2tWWlFqdXE1NVpaT3RhVHhkeUtuNURYQWdy?=
 =?utf-8?B?SXE0U1pHVDNYNlMrM1VHb3dYMk5CMXMzK0lXVGxCZWk1ZWNBVUtnOXRDNjhT?=
 =?utf-8?B?Y2wzNzVjcDljbEo5cVhrM3VIemZIZlFCTytQdDVDSjl1MEJNdWg2ODJQSGRp?=
 =?utf-8?B?VTc1bjc3Zzdvcmh6bi9MOVZFcEk0U1dqekoyWXBYc2xBNUd5Q2tFYzd2SjMr?=
 =?utf-8?B?OXRMUmR6MnVkSlJxNjRjekUyNkJJUXdrYlBzQ2tBMXlocmJub2JRdUdKSDNQ?=
 =?utf-8?B?OW5rUlhUWEN0dlFBVVhTeVg5R09uK1NuZGFBdHBwaDFDNDF3NTJWSTNxS0Za?=
 =?utf-8?B?cW5LRlFXRVNkQlpSRXdnL0RiNkxnWi83Ty9oNEViMFFraGc4VjBGSUN6YVQ1?=
 =?utf-8?B?U1FOSENNa1VtRGNKdHBNWUk1aVdCK2tycVZxTkNQWU1pa1gwMzJ0S2k2Uno1?=
 =?utf-8?B?eGVvNGVSc3RDNlFybzRyWjNYV1BlaTFBY3JxdFBVT0Fvc05VUkdjQ05kSXdj?=
 =?utf-8?B?WERER0czNUhlc3B1SjZWellhYm1RSE1ySHFkeDh3WlljVHp5Vzg3SnQxMUM2?=
 =?utf-8?B?TTgwcUNCVzVURHQwazViYUZtV1dIcGg2UTIvVjNjMjZOTDBCUHpxbkN2Q3V3?=
 =?utf-8?B?NzVIQUdJUlpjM0E0bmNPd3JaZlBUbFpyM2djeHF3a2VtVlpoRWNLN0VVblNi?=
 =?utf-8?B?Qk1vNERSR2M5b2JCeGhxVWlWS2hYWmFPZ21ZYnduTVQ0eUtTYkRXZ1llZzlm?=
 =?utf-8?B?cnp6RTdSa1NzeFBPbEZwRXNjbnAxc0Q0V293MkVkWW1LN2xXWVRoOTM3a2hw?=
 =?utf-8?B?bUZIbWhDU21OSzlwRDE5M1ZFY1ZDeUd0VStyTzQvWDdxaHNzYzgydWY2VGlW?=
 =?utf-8?B?QWRpTHRMUGVPLzZGenJoSDRWQzFad0tIU2IzY2FFRTJqSDl2cG5mL0IzNmJU?=
 =?utf-8?B?aEtHNTVYU09wdk5MclB6cGV4TlJvR00ySlZ6QmlSMWFERGxhYXBpd2ZMcWNB?=
 =?utf-8?B?YnVpTkZ0bC93NHBOdXFnNnd6R2JoeGZFcEQzRitFNGNPM3hnOFdTZ3ZrRlh1?=
 =?utf-8?B?cVFYRUZrQ3Z1cmFXL0xrc2hLc0lmZHNVYkZjMzc5M2Rpd0s4eS9pWHA1YzZt?=
 =?utf-8?B?L0lhcTVCTWwraW1jOEduMDh5UTl0NW9GR1c2UEpmNzZNcnZNaWp2THhDaDMx?=
 =?utf-8?B?K3ZNQUFuMkd0TlM4OUJLd1B1NlF6Y0t6RFB0dFE3YThMVm5TS1JmZWdnN3Jr?=
 =?utf-8?B?UjFCMVVHd0xjbEtIU0xscDJHdFZCbW1UL1VPcFFleEF1NmNHMVZKN2xjREtT?=
 =?utf-8?B?M2t5R2JNeFlEbGNZN00wcFMrMnI0NmNUWjB3bVBWaHg3ajBsc1UrcmhWeHBy?=
 =?utf-8?B?Z1RRYjhEdklHckY3dEZYQlIvUVZMNmFqaXdrNEluVk12Nzk2QzZLRjBRcjU5?=
 =?utf-8?B?QnpuOFlzRmtGNzBCZm1tU0JzWmJOS1RrTm5IdzhpNSs5TEpjdGV4MFR5L1Jn?=
 =?utf-8?B?anl1N1dMWmZXbk9RLzRpbEtOM1p3dU5ZSUt5b0psMUIvcms0cWh1VUI0blpx?=
 =?utf-8?B?cVlFNEhrV3VDcVZWWXdvUERIeExETy9YNWNjYkEvcjBqL3RhRFlnQWE2dHIr?=
 =?utf-8?B?dWNuVGxBSlVYaWR6QzlkY3F1YjkxaWwxWGEvMHZiTjdHU0ZCb2dZYi96RnlM?=
 =?utf-8?B?eTljZk5ORFIxeHlPRG8wc1lucGR6alUya0YwYjYrcHNleUVKS1N3eStIcFZI?=
 =?utf-8?B?Z1B3aVdaQU1mSFpveWNEOFNSN3VkajVGY1BFTkY5amNGZE5Wazk0MHhieFVV?=
 =?utf-8?B?QUNnM3puY2taN25oRnJwNStqNk9nT09OTVM4VVBEV3FGWThzWGpTdGgzRVh5?=
 =?utf-8?B?UmtQZTlNMXZ6Vi9UemVvOEkwRUZnNmRHWDloblluSGRCNCsvMVUvb3hJNEph?=
 =?utf-8?B?enc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 1e4bd34a-e855-4a99-b9ad-08da8e384c51
X-MS-Exchange-CrossTenant-AuthSource: DM5PR1101MB2235.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Sep 2022 05:42:47.9989
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: G6cnHRFZEbF8kwRj6TQwz2bFBunbMYTqZTqItqu5zDoohQC+miIBMr/3rNTRKkajb3xgn0IdoGU8BawFfOIsFQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4251
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 8/30/2022 10:15, Li Zhong wrote:
> e1e_rphy() could return error value when reading PHY register, which
> needs to be checked.
> 
> Signed-off-by: Li Zhong <floridsleeves@gmail.com>
> ---
>   drivers/net/ethernet/intel/e1000e/phy.c | 20 +++++++++++++++++---
>   1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c
> index fd07c3679bb1..060b263348ce 100644
> --- a/drivers/net/ethernet/intel/e1000e/phy.c
> +++ b/drivers/net/ethernet/intel/e1000e/phy.c
> @@ -2697,9 +2697,14 @@ static s32 e1000_access_phy_wakeup_reg_bm(struct e1000_hw *hw, u32 offset,
>   void e1000_power_up_phy_copper(struct e1000_hw *hw)
>   {
>   	u16 mii_reg = 0;
> +	int ret;
>   
>   	/* The PHY will retain its settings across a power down/up cycle */
> -	e1e_rphy(hw, MII_BMCR, &mii_reg);
> +	ret = e1e_rphy(hw, MII_BMCR, &mii_reg);
> +	if (ret) {
> +		e_dbg("Error reading PHY register\n");
> +		return;
> +	}
>   	mii_reg &= ~BMCR_PDOWN;
>   	e1e_wphy(hw, MII_BMCR, mii_reg);
>   }
> @@ -2715,9 +2720,14 @@ void e1000_power_up_phy_copper(struct e1000_hw *hw)
>   void e1000_power_down_phy_copper(struct e1000_hw *hw)
>   {
>   	u16 mii_reg = 0;
> +	int ret;
>   
>   	/* The PHY will retain its settings across a power down/up cycle */
> -	e1e_rphy(hw, MII_BMCR, &mii_reg);
> +	ret = e1e_rphy(hw, MII_BMCR, &mii_reg);
> +	if (ret) {
> +		e_dbg("Error reading PHY register\n");
> +		return;
> +	}
>   	mii_reg |= BMCR_PDOWN;
>   	e1e_wphy(hw, MII_BMCR, mii_reg);
>   	usleep_range(1000, 2000);
> @@ -3037,7 +3047,11 @@ s32 e1000_link_stall_workaround_hv(struct e1000_hw *hw)
>   		return 0;
>   
>   	/* Do not apply workaround if in PHY loopback bit 14 set */
> -	e1e_rphy(hw, MII_BMCR, &data);
> +	ret_val = e1e_rphy(hw, MII_BMCR, &data);
> +	if (ret_val) {
> +		e_dbg("Error reading PHY register\n");
> +		return ret_val;
> +	}
>   	if (data & BMCR_LOOPBACK)
>   		return 0;
>   
Acked-by: Sasha Neftin <sasha.neftin@intel.com>