Received-SPF: pass (google.com: domain of linux-wireless+bounces-5280-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <cf330e75-8b5e-492e-a54a-5a7bbce687e2@gmail.com>
Date: Tue, 26 Mar 2024 15:02:08 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 02/12] wifi: rtlwifi: Move code from rtl8192de to
 rtl8192d-common
To: Ping-Ke Shih <pkshih@realtek.com>,
 "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Cc: "Larry.Finger@lwfinger.net" <Larry.Finger@lwfinger.net>,
 "s.l-h@gmx.de" <s.l-h@gmx.de>, "chewitt@libreelec.tv" <chewitt@libreelec.tv>
References: <7f4b3309-1580-48f3-9426-29f1eb4052fd@gmail.com>
 <c83c2d67-d476-43c3-86e2-48bacc7d5ba2@gmail.com>
 <04c3ea92c4456641f5dd7198db4906428f2610e3.camel@realtek.com>
Content-Language: en-US
From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
In-Reply-To: <04c3ea92c4456641f5dd7198db4906428f2610e3.camel@realtek.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 22/03/2024 04:14, Ping-Ke Shih wrote:
[...]
>> +
>> +void rtl92d_get_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
>> +{
>> +       struct rtl_priv *rtlpriv = rtl_priv(hw);
>> +       struct rtl_ps_ctl *ppsc = rtl_psc(rtl_priv(hw));
>> +
>> +       switch (variable) {
>> +       case HW_VAR_RF_STATE:
>> +               *((enum rf_pwrstate *)(val)) = ppsc->rfpwr_state;
> 
> The casting of these set/get_hw_reg looks not good. If you have good ideas
> to avoid them, welcome to adjust them afterward 
> 

My idea is more of a dream: to replace the get_hw_reg and set_hw_reg
functions with many small functions, one for each HW_VAR. Most of the
drivers have very similar code in these, so they could share.

> 
>> +               break;
>> +       case HW_VAR_FWLPS_RF_ON:{
>> +               enum rf_pwrstate rfstate;
>> +               u32 val_rcr;
>> +
>> +               rtlpriv->cfg->ops->get_hw_reg(hw, HW_VAR_RF_STATE,
>> +                                             (u8 *)(&rfstate));
>> +               if (rfstate == ERFOFF) {
>> +                       *((bool *)(val)) = true;
>> +               } else {
>> +                       val_rcr = rtl_read_dword(rtlpriv, REG_RCR);
>> +                       val_rcr &= 0x00070000;
>> +                       if (val_rcr)
>> +                               *((bool *)(val)) = false;
>> +                       else
>> +                               *((bool *)(val)) = true;
>> +               }
>> +               break;
>> +       }
>>
>