Re: [Outreachy kernel] [Outreachy] [PATCH v2 2/3] staging: rtl8188eu: Line over 80 characters

[Soumyajit Deb <debsoumyajit100@gmail.com>]

On Wed, Apr 01, 2020 at 04:48:38AM +0200, Stefano Brivio wrote:
> On Wed,  1 Apr 2020 00:02:00 +0530
> Soumyajit Deb <debsoumyajit100@gmail.com> wrote:
> 
> > @@ -445,7 +455,8 @@ static void update_bmc_sta(struct adapter *padapter)
> >  		tx_ra_bitmap = 0xf;
> >  
> >  		raid = networktype_to_raid(network_type);
> > -		init_rate = get_highest_rate_idx(tx_ra_bitmap & 0x0fffffff) & 0x3f;
> > +		init_rate = get_highest_rate_idx(tx_ra_bitmap & 0x0fffffff) &
> > +							      0x3f;
> 
> Hmm, no, wait, what is 0x3f bit-wise intersected with?
> 
> a = lol;
> 
> a = lol *
>     2;
> 
> a = (lol + foo) *
>     2;
> 
> a = lol + foo *
>           2;
> 
> but not:
> 
> a = (lol + foo) *
>          2;
> 
> ...right?
>

I am sorry but I couldn't understand what you mean.

As much I can see, a bitwise AND operation is being done between 0x3f and the
return value of get_highest_rate_idx() function. The first & operation
is between the arguments of get_highest_rate_idx() function.

If the current alignment is wrong, should I place 0x3f someplace else?

But as you suggested previously in this thread, that I should place 0x3f
below the start of first & operand.

> >  
> >  		/* ap mode */
> >  		rtw_hal_set_odm_var(padapter, HAL_ODM_STA_INFO, psta, true);
> > @@ -456,7 +467,8 @@ static void update_bmc_sta(struct adapter *padapter)
> >  			arg = psta->mac_id & 0x1f;
> >  			arg |= BIT(7);
> >  			tx_ra_bitmap |= ((raid << 28) & 0xf0000000);
> > -			DBG_88E("%s, mask = 0x%x, arg = 0x%x\n", __func__, tx_ra_bitmap, arg);
> > +			DBG_88E("%s, mask = 0x%x, arg = 0x%x\n", __func__,
> > +				tx_ra_bitmap, arg);
> >  
> >  			/* bitmap[0:27] = tx_rate_bitmap */
> >  			/* bitmap[28:31]= Rate Adaptive id */
> > @@ -647,7 +659,8 @@ static void start_bss_network(struct adapter *padapter, u8 *pbuf)
> >  	rtw_hal_set_hwreg(padapter, HW_VAR_SEC_CFG, (u8 *)(&val8));
> >  
> >  	/* Beacon Control related register */
> > -	rtw_hal_set_hwreg(padapter, HW_VAR_BEACON_INTERVAL, (u8 *)(&bcn_interval));
> > +	rtw_hal_set_hwreg(padapter, HW_VAR_BEACON_INTERVAL,
> > +			  (u8 *)(&bcn_interval));
> 
> This is another pair of parentheses that you could remove: &a == (&a).
> 
> -- 
> Stefano

I noticed those parentheses while removing other extra parentheses
but I didn't remove these as most of these parentheses are present
after a type cast.
Will it not look confusing if I remove these paretheses?
If not, I will remove them :)

Thank you.

--
Soumyajit
>