RE: [PATCH] staging: r8188eu: remove local BIT macro

[David Laight]

From: Dan Carpenter
> Sent: 21 March 2022 09:07
> 
> On Sat, Mar 19, 2022 at 10:35:58PM +0000, David Laight wrote:
> > OTOH this code is a big pile of poo.
> > Abstraction functions gone mad.
> 
> Yeah.
> 
> I wrote an email similar to yours and I even wrote some sample code.
> But then I deleted it because I don't pay Martin anything so I'm just
> grateful for what he sends and can't reasonably ask for more.
> 
> This code constantly amazes me with how many abstractions there are.
> Martin keeps deleting them, and I think he's going to run out but so
> far that hasn't happened.
> 
> Anyway here is the diff just for laughs since you brought it up.  Not
> something that's necessarry and definitely not a priority.
> 
> I don't really like enable/disable functions that do opposite things
> depending on if true/false is passed as a parameter.  They're normally
> more readable split apart.
> 
> Ideally there would be adapter_to_pdbm() and adapter_to_podm() helper
> functions.
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c
> b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> index 665b077190bc..d973cf341031 100644
> --- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
> +++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
> @@ -276,12 +276,23 @@ void Restore_DM_Func_Flag(struct adapter *padapter)
>  	SetHwReg8188EU(padapter, HW_VAR_DM_FUNC_OP, (u8 *)(&saveflag));
>  }
> 
> +void enable_dm_func(struct adapter *padapter, u32 mode)
> + {
> +	struct dm_pri *pdmpriv = adapter_to_pdbm(padapter);
> +	struct odm_dm_struct *podmpriv = adapter_to_pod(padapter);
> +
> +	if (mode == DYNAMIC_ALL_FUNC_ENABLE) {
> +		podmpriv->SupportAbility = pdmpriv->InitODMFlag;
> +	} else {
> +		podmpriv->SupportAbility |= mode;
> +	}
> +}
> +
> +void disable_dm_func(struct adapter *padapter, u32 mode)
> +{
> +	struct odm_dm_struct *podmpriv = adapter_to_pod(padapter);
> +
> +	podmpriv->SupportAbility &= mode;
> + }

I'd go even further.
One function for each of 'set', 'enable' and 'disable'.
Doing '=', '|=' and '&= ~'.

But then you realise it just isn't worth the effort.
Just type:
		adapter_to_pod(padapter)-> SupportAbility = ...
directly in the code.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)