From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 63866C43217 for ; Thu, 3 Nov 2022 08:20:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230521AbiKCIUA (ORCPT ); Thu, 3 Nov 2022 04:20:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34548 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229551AbiKCIT5 (ORCPT ); Thu, 3 Nov 2022 04:19:57 -0400 Received: from msg-4.mailo.com (msg-4.mailo.com [213.182.54.15]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6A8EF6387 for ; Thu, 3 Nov 2022 01:19:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=mailo.com; s=mailo; t=1667463587; bh=Q1gtZhNcPAX6xPcOwW6C1nKDG54ohaBKBclCqQEXMWk=; h=X-EA-Auth:Date:From:To:Cc:Subject:Message-ID:References: MIME-Version:Content-Type:In-Reply-To; b=XfiMZgjzTJCbMdf2XBXxvyuOpQLjeHMnDB6uR+0SgX2SkvgmOFfaqVKH16vgzfuin tEXqanqbE4eI+Vz4I8thlwW94GnLWJntlvnBlA/WZ6X8x7JQFQBfa1lNgLU3r0DPG+ +vRa1NZeTx2B7+zDmuR+istc/O8UKzzArDh/y3y4= Received: by b-1.in.mailobj.net [192.168.90.11] with ESMTP via [213.182.55.206] Thu, 3 Nov 2022 09:19:47 +0100 (CET) X-EA-Auth: fvalj0XVWmHSXWkNpFkJL08fpLTtLNaG1NjteZ/jLJzTnmY2SmvlE1rmcvDFi8TXOF516L1c6KrAqLYXA9LD+P6zPCocubVs Date: Thu, 3 Nov 2022 13:49:42 +0530 From: Deepak R Varma To: Julia Lawall Cc: outreachy@lists.linux.dev, Greg Kroah-Hartman , linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging: rtl8723bs: Use min/max macros for variable comparison Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Nov 03, 2022 at 08:55:32AM +0100, Julia Lawall wrote: > > > On Thu, 3 Nov 2022, Deepak R Varma wrote: > > > Simplify code by using min and max helper macros in place of lengthy > > if/else block oriented logical evaluation and value assignment. This > > issue is identified by coccicheck using the minmax.cocci file. > > > > Signed-off-by: Deepak R Varma > > --- > > > > Please note: > > 1. Using min for max_AMPDU_len computation warning was NOT auto generated by > > the cocciecheck command. This was caught while surround code review and > > was manually changed. > > 2. Checkpatch script continues to complaint about min_MPDU_spacing > > computation line being more than 100 character in length. I did not find a > > better formatting that will address this checkpatch warning. Any > > suggestions are most welcome. > > 3. Proposed changes are compile tested only on my x86 based VM. > > > > > > drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 12 ++++-------- > > drivers/staging/rtl8723bs/hal/odm_DIG.c | 5 +---- > > 2 files changed, 5 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c > > index 18ba846c0b7b..dcda587b84bc 100644 > > --- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c > > +++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c > > @@ -986,15 +986,11 @@ void HT_caps_handler(struct adapter *padapter, struct ndis_80211_var_ie *pIE) > > pmlmeinfo->HT_caps.u.HT_cap[i] &= (pIE->data[i]); > > } else { > > /* modify from fw by Thomas 2010/11/17 */ > > - if ((pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x3) > (pIE->data[i] & 0x3)) > > - max_AMPDU_len = (pIE->data[i] & 0x3); > > - else > > - max_AMPDU_len = (pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x3); > > + max_AMPDU_len = min((pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x3), > > + (pIE->data[i] & 0x3)); > > > > - if ((pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x1c) > (pIE->data[i] & 0x1c)) > > - min_MPDU_spacing = (pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x1c); > > - else > > - min_MPDU_spacing = (pIE->data[i] & 0x1c); > > + min_MPDU_spacing = max((pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x1c), > > + (pIE->data[i] & 0x1c)); > > There seem to be a lot of unnecessary parentheses. Admittedly they were > there before, but since you are creating the max call from scratch in this > patch, perhaps the parentheses around the arguments could be dropped at > the same time. Sounds good. I will improve further and send in a revision. Also, do you have a comment on why coccicheck did not catch the min opportunity for max_AMPDU_len computation? I am not seeing anything different with the if/else blocks here. Thank you, ./drv > > julia > > > > > pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para = max_AMPDU_len | min_MPDU_spacing; > > } > > diff --git a/drivers/staging/rtl8723bs/hal/odm_DIG.c b/drivers/staging/rtl8723bs/hal/odm_DIG.c > > index 07edf74ccfe5..97a51546463a 100644 > > --- a/drivers/staging/rtl8723bs/hal/odm_DIG.c > > +++ b/drivers/staging/rtl8723bs/hal/odm_DIG.c > > @@ -598,10 +598,7 @@ void odm_DIGbyRSSI_LPS(void *pDM_VOID) > > /* Lower bound checking */ > > > > /* RSSI Lower bound check */ > > - if ((pDM_Odm->RSSI_Min-10) > DM_DIG_MIN_NIC) > > - RSSI_Lower = pDM_Odm->RSSI_Min-10; > > - else > > - RSSI_Lower = DM_DIG_MIN_NIC; > > + RSSI_Lower = max(pDM_Odm->RSSI_Min - 10, DM_DIG_MIN_NIC); > > > > /* Upper and Lower Bound checking */ > > if (CurrentIGI > DM_DIG_MAX_NIC) > > -- > > 2.34.1 > > > > > > > > > > >