Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 2 Apr 2021 14:40:46 +0200
From:   Fabio Aiuto <fabioaiuto83@gmail.com>
To:     Dan Carpenter <dan.carpenter@oracle.com>
Cc:     gregkh@linuxfoundation.org, joe@perches.com,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 01/16] staging: rtl8723bs: remove RT_TRACE logs in
 core/rtw_xmit.c
Message-ID: <20210402124045.GA1420@agape.jhs>
References: <cover.1617356821.git.fabioaiuto83@gmail.com>
 <72ab5222629f912ca2dbe825d194108992d321e7.1617356821.git.fabioaiuto83@gmail.com>
 <20210402115626.GV2088@kadam>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210402115626.GV2088@kadam>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Fri, Apr 02, 2021 at 02:56:26PM +0300, Dan Carpenter wrote:
> On Fri, Apr 02, 2021 at 12:01:21PM +0200, Fabio Aiuto wrote:
> > @@ -568,20 +561,11 @@ static s32 update_attrib_sec_info(struct adapter *padapter, struct pkt_attrib *p
> >  	if (pattrib->encrypt > 0)
> >  		memcpy(pattrib->dot118021x_UncstKey.skey, psta->dot118021x_UncstKey.skey, 16);
> >  
> > -	RT_TRACE(_module_rtl871x_xmit_c_, _drv_info_,
> > -		("update_attrib: encrypt =%d  securitypriv.sw_encrypt =%d\n",
> > -		pattrib->encrypt, padapter->securitypriv.sw_encrypt));
> > -
> >  	if (pattrib->encrypt &&
> > -		((padapter->securitypriv.sw_encrypt == true) || (psecuritypriv->hw_decrypted == false))) {
> > +		((padapter->securitypriv.sw_encrypt) || (!psecuritypriv->hw_decrypted)))
> 
> You've done too much clean up here.  Just remove the { but leave the
> == true/false comparisons.
> 
> If the patch is only changing five lines or code then fixing checkpatch
> warnings on the line of code you are changing is fine, but in this case
> you're doing a bunch of changes and these sort of cleanups make it hard
> to review.
> 
> Ease to spot that the curly brace changed:
> -		((padapter->securitypriv.sw_encrypt == true) || (psecuritypriv->hw_decrypted == false))) {
> +		((padapter->securitypriv.sw_encrypt == true) || (psecuritypriv->hw_decrypted == false)))
> 
> Hard to spot:
> -		((padapter->securitypriv.sw_encrypt == true) || (psecuritypriv->hw_decrypted == false))) {
> +		((padapter->securitypriv.sw_encrypt) || (!psecuritypriv->hw_decrypted)))
> 
> regards,
> dan carpenter
> 

thank you Dan, it's a good tuning process for me. Shall I resend the
whole patchset? Maybe some of them are ok...

fabio