On Wed, 2024-03-20 at 21:34 +0200, Bitterblue Smith wrote:
>
> Create the new module rtl8192d-common and move some code into it from
> rtl8192de. Now the rtl8192de driver (PCI) and the new rtl8192du driver
> (USB) can share some of the code.
>
> This is mostly the code that required little effort to make it
> shareable. There are a few more functions which they could share, with
> more changes.
>
> Add phy_iq_calibrate member to struct rtl_hal_ops to allow moving the
> TX power tracking code from dm.c.
>
> The other changes in this patch are adjusting whitespace, renaming some
> functions, making some arrays const, and making checkpatch.pl less
> unhappy.
>
> rtl8192de is compile-tested only. rtl8192d-common is tested with the
> new rtl8192du driver.
Unfortunately I don't have a rtl8192de for testing neither...
>
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
> ---
> v3:
> - Add the declarations of rtl92d_acquire_cckandrw_pagea_ctl and
> rtl92d_release_cckandrw_pagea_ctl in phy_common.h in order to fix
> sparse warnings about locks.
I am not clear the difference from v2 to v3, but super thank to fix that
because sparse/smatch warnings free is the basic requirement of WiFi driver.
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.h
> b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.h
> new file mode 100644
> index 000000000000..5e21c6a1d1bc
> --- /dev/null
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/dm_common.h
> @@ -0,0 +1,100 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2009-2012 Realtek Corporation.*/
> +
> +#ifndef __RTL92D_DM_COMMON_H__
a space between '#ifndef' and name.(be consistent with below statement)
> +#define __RTL92D_DM_COMMON_H__
> +
[...]
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192d/fw_common.h
> b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/fw_common.h
> new file mode 100644
> index 000000000000..bf3add89be87
> --- /dev/null
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/fw_common.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2009-2012 Realtek Corporation.*/
> +
> +#ifndef __RTL92D__FW_COMMON_H__
> +#define __RTL92D__FW_COMMON_H__
> +
> +#define FW_8192D_START_ADDRESS 0x1000
> +#define FW_8192D_PAGE_SIZE 4096
> +#define FW_8192D_POLLING_TIMEOUT_COUNT 1000
> +
> +#define IS_FW_HEADER_EXIST(_pfwhdr) \
> + ((GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFF0) == 0x92C0 || \
> + (GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFF0) == 0x88C0 || \
> + (GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFFF) == 0x92D0 || \
> + (GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFFF) == 0x92D1 || \
> + (GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFFF) == 0x92D2 || \
> + (GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFFF) == 0x92D3)
It would looks better to align parentheses
#define IS_FW_HEADER_EXIST(_pfwhdr) \
((GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFF0) == 0x92C0 || \
(GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFF0) == 0x88C0 || \
(GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFFF) == 0x92D0 || \
(GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFFF) == 0x92D1 || \
(GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFFF) == 0x92D2 || \
(GET_FIRMWARE_HDR_SIGNATURE(_pfwhdr) & 0xFFFF) == 0x92D3)
> +
> +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
> + 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;
> + }
>
[...]
> +void rtl92de_set_key(struct ieee80211_hw *hw, u32 key_index,
> + u8 *p_macaddr, bool is_group, u8 enc_algo,
> + bool is_wepkey, bool clear_all)
> +{
> + struct rtl_priv *rtlpriv = rtl_priv(hw);
> + struct rtl_mac *mac = rtl_mac(rtl_priv(hw));
> + struct rtl_efuse *rtlefuse = rtl_efuse(rtl_priv(hw));
> + u8 *macaddr = p_macaddr;
> + u32 entry_id;
> + bool is_pairwise = false;
> + static u8 cam_const_addr[4][6] = {
> + {0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
> + {0x00, 0x00, 0x00, 0x00, 0x00, 0x01},
> + {0x00, 0x00, 0x00, 0x00, 0x00, 0x02},
> + {0x00, 0x00, 0x00, 0x00, 0x00, 0x03}
> + };
> + static u8 cam_const_broad[] = {
> + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff
> + };
static const for these two tables.
> +
> +static u8 _rtl92d_phy_get_chnlgroup_bypg(u8 chnlindex)
> +{
> + u8 group;
> + u8 channel_info[59] = {
> + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14,
> + 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58,
> + 60, 62, 64, 100, 102, 104, 106, 108, 110, 112,
> + 114, 116, 118, 120, 122, 124, 126, 128, 130, 132,
> + 134, 136, 138, 140, 149, 151, 153, 155, 157, 159,
> + 161, 163, 165
> + };
This table looks identical to 'static const u8 channel_all[59]'
> +
> + if (channel_info[chnlindex] <= 3) /* Chanel 1-3 */
> + group = 0;
> + else if (channel_info[chnlindex] <= 9) /* Channel 4-9 */
> + group = 1;
> + else if (channel_info[chnlindex] <= 14) /* Channel 10-14 */
> + group = 2;
> + else if (channel_info[chnlindex] <= 64)
> + group = 6;
> + else if (channel_info[chnlindex] <= 140)
> + group = 7;
> + else
> + group = 8;
> + return group;
> +}
> +
[...]
> +
> +void rtl92de_set_desc(struct ieee80211_hw *hw, u8 *pdesc8, bool istx,
> + u8 desc_name, u8 *val)
> +{
> + __le32 *pdesc = (__le32 *)pdesc8;
only need one space between '__le32' and "*pdesc'.
> +
> +struct rx_fwinfo_92d {
> + u8 gain_trsw[4];
> + u8 pwdb_all;
> + u8 cfosho[4];
> + u8 cfotail[4];
> + s8 rxevm[2];
> + s8 rxsnr[4];
> + u8 pdsnr[2];
> + u8 csi_current[2];
> + u8 csi_target[2];
> + u8 sigevm;
> + u8 max_ex_pwr;
> + u8 ex_intf_flag:1;
> + u8 sgi_en:1;
> + u8 rxsc:2;
> + u8 reserve:4;
These bits are not usable for big-endian machine. Let's add to our
TODO list.
> +} __packed;
> +
> +struct rx_desc_92d {
> + u32 length:14;
> + u32 crc32:1;
> + u32 icverror:1;
> + u32 drv_infosize:4;
> + u32 security:3;
> + u32 qos:1;
> + u32 shift:2;
> + u32 phystatus:1;
> + u32 swdec:1;
> + u32 lastseg:1;
> + u32 firstseg:1;
> + u32 eor:1;
> + u32 own:1;
> +
> + u32 macid:5;
> + u32 tid:4;
> + u32 hwrsvd:5;
> + u32 paggr:1;
> + u32 faggr:1;
> + u32 a1_fit:4;
> + u32 a2_fit:4;
> + u32 pam:1;
> + u32 pwr:1;
> + u32 moredata:1;
> + u32 morefrag:1;
> + u32 type:2;
> + u32 mc:1;
> + u32 bc:1;
> +
> + u32 seq:12;
> + u32 frag:4;
> + u32 nextpktlen:14;
> + u32 nextind:1;
> + u32 rsvd:1;
> +
> + u32 rxmcs:6;
> + u32 rxht:1;
> + u32 amsdu:1;
> + u32 splcp:1;
> + u32 bandwidth:1;
> + u32 htc:1;
> + u32 tcpchk_rpt:1;
> + u32 ipcchk_rpt:1;
> + u32 tcpchk_valid:1;
> + u32 hwpcerr:1;
> + u32 hwpcind:1;
> + u32 iv0:16;
> +
> + u32 iv1;
> +
> + u32 tsfl;
> +
> + u32 bufferaddress;
> + u32 bufferaddress64;
> +
> +} __packed;
Please correct this CHECK reported by checkpatch.pl
CHECK: No space is necessary after a cast
#2675: FILE: drivers/net/wireless/realtek/rtlwifi/rtl8192d/hw_common.c:974:
+ rtl92d_fill_h2c_cmd(hw, H2C_RA_MASK, 5, (u8 *) value);
