public inbox for linux-wireless@vger.kernel.org
 help / color / mirror / Atom feed
From: Ping-Ke Shih <pkshih@realtek.com>
To: Bitterblue Smith <rtl8821cerfe2@gmail.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Cc: Larry Finger <Larry.Finger@lwfinger.net>,
	Stefan Lippers-Hollmann <s.l-h@gmx.de>,
	Christian Hewitt <chewitt@libreelec.tv>
Subject: RE: [PATCH v5 04/11] wifi: rtlwifi: Add rtl8192du/phy.{c,h}
Date: Fri, 10 May 2024 03:43:28 +0000	[thread overview]
Message-ID: <4ab7446ccf754f45a9c1f85a22b57fdf@realtek.com> (raw)
In-Reply-To: <f62149e8-a980-40d2-a3b2-97b5886938fa@gmail.com>

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> These contain mostly the calibration and channel switching routines
> for RTL8192DU.
> 
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> ---
>  .../wireless/realtek/rtlwifi/rtl8192du/phy.c  | 3181 +++++++++++++++++
>  .../wireless/realtek/rtlwifi/rtl8192du/phy.h  |   32 +
>  2 files changed, 3213 insertions(+)
>  create mode 100644 drivers/net/wireless/realtek/rtlwifi/rtl8192du/phy.c
>  create mode 100644 drivers/net/wireless/realtek/rtlwifi/rtl8192du/phy.h
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192du/phy.c
> b/drivers/net/wireless/realtek/rtlwifi/rtl8192du/phy.c
> new file mode 100644
> index 000000000000..5999997f4ef9
> --- /dev/null
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192du/phy.c

[...]

> +
> +/* [mode][patha+b][reg] */

There is only one mode. How about shrinking dimension from 3 to 2?

> +static const u32 rf_imr_param_normal[1][3][MAX_RF_IMR_INDEX_NORMAL] = {
> +       {
> +               /* channel 1-14. */
> +               {
> +                       0x70000, 0x00ff0, 0x4400f, 0x00ff0, 0x0, 0x0, 0x0,
> +                       0x0, 0x0, 0x64888, 0xe266c, 0x00090, 0x22fff
> +               },
> +               /* path 36-64 */
> +               {
> +                       0x70000, 0x22880, 0x4470f, 0x55880, 0x00070, 0x88000,
> +                       0x0, 0x88080, 0x70000, 0x64a82, 0xe466c, 0x00090,
> +                       0x32c9a
> +               },
> +               /* 100 -165 */
> +               {
> +                       0x70000, 0x44880, 0x4477f, 0x77880, 0x00070, 0x88000,
> +                       0x0, 0x880b0, 0x0, 0x64b82, 0xe466c, 0x00090, 0x32c9a
> +               }
> +       }
> +};
> +

[...]


> +static bool _rtl92d_phy_config_bb_with_headerfile(struct ieee80211_hw *hw,
> +                                                 u8 configtype)
> +{

[...]

> +       } else if (configtype == BASEBAND_CONFIG_AGC_TAB) {
> +               if (rtlhal->interfaceindex == 0) {
> +                       for (i = 0; i < agctab_arraylen; i = i + 2) {
> +                               rtl_set_bbreg(hw, agctab_array_table[i],
> +                                             MASKDWORD,
> +                                             agctab_array_table[i + 1]);
> +                               /* Add 1us delay between BB/RF register
> +                                * setting.
> +                                */
> +                               udelay(1);
> +                               rtl_dbg(rtlpriv, COMP_INIT, DBG_TRACE,
> +                                       "The Rtl819XAGCTAB_Array_Table[0] is %u Rtl819XPHY_REGArray[1]
> is %u\n",
> +                                       agctab_array_table[i],
> +                                       agctab_array_table[i + 1]);
> +                       }
> +                       rtl_dbg(rtlpriv, COMP_INIT, DBG_LOUD,
> +                               "Normal Chip, MAC0, load Rtl819XAGCTAB_Array\n");
> +               } else {
> +                       if (rtlhal->current_bandtype == BAND_ON_2_4G) {
> +                               for (i = 0; i < agctab_arraylen; i = i + 2) {
> +                                       rtl_set_bbreg(hw, agctab_array_table[i],
> +                                                     MASKDWORD,
> +                                                     agctab_array_table[i + 1]);
> +                                       /* Add 1us delay between BB/RF register
> +                                        * setting.
> +                                        */
> +                                       udelay(1);
> +                                       rtl_dbg(rtlpriv, COMP_INIT, DBG_TRACE,
> +                                               "The Rtl819XAGCTAB_Array_Table[0] is %u
> Rtl819XPHY_REGArray[1] is %u\n",
> +                                               agctab_array_table[i],
> +                                               agctab_array_table[i + 1]);
> +                               }
> +                               rtl_dbg(rtlpriv, COMP_INIT, DBG_LOUD,
> +                                       "Load Rtl819XAGCTAB_2GArray\n");
> +                       } else {
> +                               for (i = 0; i < agctab_5garraylen; i = i + 2) {
> +                                       rtl_set_bbreg(hw,
> +                                                     agctab_5garray_table[i],
> +                                                     MASKDWORD,
> +                                                     agctab_5garray_table[i + 1]);
> +                                       /* Add 1us delay between BB/RF register
> +                                        * setting.
> +                                        */
> +                                       udelay(1);
> +                                       rtl_dbg(rtlpriv, COMP_INIT, DBG_TRACE,
> +                                               "The Rtl819XAGCTAB_5GArray_Table[0] is %u
> Rtl819XPHY_REGArray[1] is %u\n",
> +                                               agctab_5garray_table[i],
> +                                               agctab_5garray_table[i + 1]);
> +                               }
> +                               rtl_dbg(rtlpriv, COMP_INIT, DBG_LOUD,
> +                                       "Load Rtl819XAGCTAB_5GArray\n");
> +                       }

Three blocks are very similar, only arrays are different. Can you change them
to
    if (inf == 0) {
        array = xxx;
        array_size = xxx_len;
    } else {
        if (2ghz) {
            array = yyy_2ghz;
            array_size = yyy_2ghz_len;
        } else {
            array = yyy_5ghz;
            array_size = yyy_5ghz_len;
        }
    }

    for (i = 0; i < array_size; i += 2) {
        rtl_set_bbreg(...)
        udelay(1);
    }

> +               }
> +       }
> +       return true;
> +}

[...]

> +void rtl92d_phy_set_bw_mode(struct ieee80211_hw *hw,
> +                           enum nl80211_channel_type ch_type)
> +{
> +       struct rtl_priv *rtlpriv = rtl_priv(hw);
> +       struct rtl_hal *rtlhal = rtl_hal(rtlpriv);
> +       struct rtl_phy *rtlphy = &rtlpriv->phy;
> +       struct rtl_mac *mac = rtl_mac(rtlpriv);
> +       u8 reg_bw_opmode;
> +       u8 reg_prsr_rsc;
> +
> +       if (rtlphy->set_bwmode_inprogress)
> +               return;
> +       if ((is_hal_stop(rtlhal)) || (RT_CANNOT_IO(hw))) {
> +               rtl_dbg(rtlpriv, COMP_ERR, DBG_WARNING,
> +                       "FALSE driver sleep or unload\n");
> +               return;
> +       }
> +
> +       rtlphy->set_bwmode_inprogress = true;
> +
> +       rtl_dbg(rtlpriv, COMP_SCAN, DBG_TRACE, "Switch to %s bandwidth\n",
> +               rtlphy->current_chan_bw == HT_CHANNEL_WIDTH_20 ?
> +               "20MHz" : "40MHz");
> +
> +       reg_bw_opmode = rtl_read_byte(rtlpriv, REG_BWOPMODE);
> +       reg_prsr_rsc = rtl_read_byte(rtlpriv, REG_RRSR + 2);
> +
> +       switch (rtlphy->current_chan_bw) {
> +       case HT_CHANNEL_WIDTH_20:
> +               reg_bw_opmode |= BW_OPMODE_20MHZ;
> +               rtl_write_byte(rtlpriv, REG_BWOPMODE, reg_bw_opmode);
> +               break;
> +       case HT_CHANNEL_WIDTH_20_40:
> +               reg_bw_opmode &= ~BW_OPMODE_20MHZ;
> +               rtl_write_byte(rtlpriv, REG_BWOPMODE, reg_bw_opmode);
> +
> +               reg_prsr_rsc = (reg_prsr_rsc & 0x90) |
> +                       (mac->cur_40_prime_sc << 5);

nit:

reg_prsr_rsc = (reg_prsr_rsc & 0x90) |
               (mac->cur_40_prime_sc << 5);

> +               rtl_write_byte(rtlpriv, REG_RRSR + 2, reg_prsr_rsc);
> +               break;
> +       default:
> +               pr_err("unknown bandwidth: %#X\n",
> +                      rtlphy->current_chan_bw);
> +               break;
> +       }
> +
> +       switch (rtlphy->current_chan_bw) {
> +       case HT_CHANNEL_WIDTH_20:
> +               rtl92d_phy_set_bb_reg_1byte(hw, RFPGA0_RFMOD, BRFMOD, 0x0);
> +               rtl_set_bbreg(hw, RFPGA1_RFMOD, BRFMOD, 0x0);
> +               /* SET BIT10 BIT11  for receive cck */
> +               rtl_set_bbreg(hw, RFPGA0_ANALOGPARAMETER2, BIT(10) |
> +                             BIT(11), 3);

Breaking mask argument is not good. Suggest

		rtl_set_bbreg(hw, RFPGA0_ANALOGPARAMETER2, BIT(10) | BIT(11), 3);

> +               break;
> +       case HT_CHANNEL_WIDTH_20_40:
> +               rtl92d_phy_set_bb_reg_1byte(hw, RFPGA0_RFMOD, BRFMOD, 0x1);
> +               rtl_set_bbreg(hw, RFPGA1_RFMOD, BRFMOD, 0x1);
> +               /* Set Control channel to upper or lower.
> +                * These settings are required only for 40MHz
> +                */
> +               if (rtlhal->current_bandtype == BAND_ON_2_4G)
> +                       rtl_set_bbreg(hw, RCCK0_SYSTEM, BCCKSIDEBAND,
> +                                     mac->cur_40_prime_sc >> 1);
> +               rtl_set_bbreg(hw, ROFDM1_LSTF, 0xC00, mac->cur_40_prime_sc);
> +               /* SET BIT10 BIT11  for receive cck */
> +               rtl_set_bbreg(hw, RFPGA0_ANALOGPARAMETER2,
> +                             BIT(10) | BIT(11), 0);
> +               rtl_set_bbreg(hw, 0x818, (BIT(26) | BIT(27)),
> +                             (mac->cur_40_prime_sc ==
> +                             HAL_PRIME_CHNL_OFFSET_LOWER) ? 2 : 1);

Need one space in front of HAL_PRIME_CHNL_OFFSET_LOWER, but parenthesis
is unnecessary, so

rtl_set_bbreg(hw, 0x818, (BIT(26) | BIT(27)),
              mac->cur_40_prime_sc ==
              HAL_PRIME_CHNL_OFFSET_LOWER ? 2 : 1);


> +               break;
> +       default:
> +               pr_err("unknown bandwidth: %#X\n",
> +                      rtlphy->current_chan_bw);
> +               break;
> +       }
> +
> +       rtl92d_phy_rf6052_set_bandwidth(hw, rtlphy->current_chan_bw);
> +
> +       rtlphy->set_bwmode_inprogress = false;
> +       rtl_dbg(rtlpriv, COMP_SCAN, DBG_TRACE, "<==\n");
> +}
> +

[...]

> +static void _rtl92d_phy_switch_rf_setting(struct ieee80211_hw *hw, u8 channel)
> +{
> +       struct rtl_usb *rtlusb = rtl_usbdev(rtl_usbpriv(hw));
> +       struct rtl_priv *rtlpriv = rtl_priv(hw);
> +       struct rtl_hal *rtlhal = &rtlpriv->rtlhal;
> +       struct rtl_phy *rtlphy = &rtlpriv->phy;
> +       u8 path = rtlhal->current_bandtype == BAND_ON_5G ? RF90_PATH_A
> +                                                        : RF90_PATH_B;
> +       u32 u4regvalue, mask = 0x1C000, value = 0, u4tmp, u4tmp2;
> +       bool need_pwr_down = false, internal_pa = false;
> +       u32 regb30 = rtl_get_bbreg(hw, 0xb30, BIT(27));
> +       u8 index = 0, i = 0, rfpath = RF90_PATH_A;

initializers of i and rfpath are unnecessary. 

[...]

> +static void _rtl92d_phy_patha_fill_iqk_matrix(struct ieee80211_hw *hw,
> +                                             bool iqk_ok, long result[][8],
> +                                             u8 final_candidate, bool txonly)
> +{
> +       struct rtl_priv *rtlpriv = rtl_priv(hw);
> +       struct rtl_hal *rtlhal = &rtlpriv->rtlhal;
> +       u32 oldval_0, val_x, tx0_a, reg;
> +       long val_y, tx0_c;
> +       bool is2t = IS_92D_SINGLEPHY(rtlhal->version) ||
> +                   rtlhal->macphymode == DUALMAC_DUALPHY;
> +
> +       if (rtlhal->current_bandtype == BAND_ON_5G) {
> +               _rtl92d_phy_patha_fill_iqk_matrix_5g_normal(hw, iqk_ok, result,
> +                                                           final_candidate,
> +                                                           txonly);
> +               return;
> +       }
> +
> +       RTPRINT(rtlpriv, FINIT, INIT_IQK,
> +               "Path A IQ Calibration %s !\n", iqk_ok ? "Success" : "Failed");
> +       if (final_candidate == 0xFF) {
> +               return;
> +       } else if (iqk_ok) {

if (final_candidate == 0xFF || !iqk_ok)
    return;

Reduce one level indent for following statements.

(similar pattern to _rtl92d_phy_pathb_fill_iqk_matrix)

[...]


> +void rtl92d_phy_set_poweron(struct ieee80211_hw *hw)
> +{
> +       struct rtl_priv *rtlpriv = rtl_priv(hw);
> +       struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
> +       u32 mac_reg = (rtlhal->interfaceindex == 0 ? REG_MAC0 : REG_MAC1);
> +       u8 value8;
> +       u16 i;
> +
> +       /* notice fw know band status  0x81[1]/0x53[1] = 0: 5G, 1: 2G */
> +       if (rtlhal->current_bandtype == BAND_ON_2_4G) {
> +               value8 = rtl_read_byte(rtlpriv, mac_reg);
> +               value8 |= BIT(1);
> +               rtl_write_byte(rtlpriv, mac_reg, value8);
> +       } else {
> +               value8 = rtl_read_byte(rtlpriv, mac_reg);
> +               value8 &= (~BIT(1));

no need parenthesis around ~BIT(1).

[...]


  reply	other threads:[~2024-05-10  3:43 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-08 10:46 [PATCH v5 00/11] wifi: rtlwifi: Add new rtl8192du driver Bitterblue Smith
2024-05-08 10:47 ` [PATCH v5 01/11] wifi: rtlwifi: Add rtl8192du/table.{c,h} Bitterblue Smith
2024-05-10  1:54   ` Ping-Ke Shih
2024-05-08 10:48 ` [PATCH v5 02/11] wifi: rtlwifi: Add new members to struct rtl_priv for RTL8192DU Bitterblue Smith
2024-05-10  2:14   ` Ping-Ke Shih
2024-05-08 10:48 ` [PATCH v5 03/11] wifi: rtlwifi: Add rtl8192du/hw.{c,h} Bitterblue Smith
2024-05-10  3:04   ` Ping-Ke Shih
2024-05-12 14:34     ` Bitterblue Smith
2024-05-13  6:29       ` Ping-Ke Shih
2024-05-08 10:49 ` [PATCH v5 04/11] wifi: rtlwifi: Add rtl8192du/phy.{c,h} Bitterblue Smith
2024-05-10  3:43   ` Ping-Ke Shih [this message]
2024-05-08 10:50 ` [PATCH v5 05/11] wifi: rtlwifi: Add rtl8192du/trx.{c,h} Bitterblue Smith
2024-05-08 10:51 ` [PATCH v5 06/11] wifi: rtlwifi: Add rtl8192du/rf.{c,h} Bitterblue Smith
2024-05-08 10:52 ` [PATCH v5 07/11] wifi: rtlwifi: Add rtl8192du/fw.{c,h} and rtl8192du/led.{c,h} Bitterblue Smith
2024-05-08 10:53 ` [PATCH v5 08/11] wifi: rtlwifi: Add rtl8192du/dm.{c,h} Bitterblue Smith
2024-05-10  3:51   ` Ping-Ke Shih
2024-05-08 10:53 ` [PATCH v5 09/11] wifi: rtlwifi: Constify rtl_hal_cfg.{ops,usb_interface_cfg} and rtl_priv.cfg Bitterblue Smith
2024-05-08 10:54 ` [PATCH v5 10/11] wifi: rtlwifi: Add rtl8192du/sw.{c,h} Bitterblue Smith
2024-05-10  5:52   ` Ping-Ke Shih
2024-05-08 10:55 ` [PATCH v5 11/11] wifi: rtlwifi: Enable the new rtl8192du driver Bitterblue Smith
2024-05-10  5:46   ` kernel test robot
2024-05-10  5:57   ` Ping-Ke Shih
2024-05-10  4:00 ` [PATCH v5 00/11] wifi: rtlwifi: Add " Stefan Lippers-Hollmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4ab7446ccf754f45a9c1f85a22b57fdf@realtek.com \
    --to=pkshih@realtek.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=chewitt@libreelec.tv \
    --cc=linux-wireless@vger.kernel.org \
    --cc=rtl8821cerfe2@gmail.com \
    --cc=s.l-h@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox