RE: [PATCH rtw-next v2 2/3] wifi: rtlwifi: convert pci if-statement to ID table

[Ping-Ke Shih]

William Hansen-Baird <william.hansen.baird@xxxxxxxxx> wrote:
> Refactor the ASUSTek quirk logic from an if-statement to a standard
> rtl_aspm_quirks pci_device_id table. This allows future devices with
> the same quirk to be added more easily while avoiding a large if-chain.
> 
> Signed-off-by: William Hansen-Baird <william.hansen.baird@xxxxxxxxx>
> ---
>  drivers/net/wireless/realtek/rtlwifi/pci.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c
> b/drivers/net/wireless/realtek/rtlwifi/pci.c
> index a5bab06f2b61..9de396879806 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
> @@ -31,6 +31,11 @@ static const u8 ac_to_hwq[] = {
>         BK_QUEUE
>  };
> 
> +static const struct pci_device_id rtl_aspm_quirks[] = {
> +       { PCI_DEVICE_SUB(PCI_VENDOR_ID_REALTEK, 0xb723, 0x11ad, 0x1723) },

Would you leave a comment ASUSTek F441U/X555UQ?

> +       { 0 }

Just "{}"

> +};
> +
>  static u8 _rtl_mac_to_hwqueue(struct ieee80211_hw *hw, struct sk_buff *skb)
>  {
>         struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
> @@ -328,21 +333,17 @@ static void rtl_pci_parse_configuration(struct pci_dev *pdev,
> 
>  static void rtl_pci_init_aspm(struct ieee80211_hw *hw)
>  {
> -       struct rtl_priv *rtlpriv = rtl_priv(hw);
>         struct rtl_ps_ctl *ppsc = rtl_psc(rtl_priv(hw));
>         struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
> 
>         _rtl_pci_update_default_setting(hw);
> 
> -       /* RTL8723BE found on some ASUSTek laptops, such as F441U and
> -        * X555UQ with subsystem ID 11ad:1723 are known to output large
> +       /* Certain pci devices are known to output large

Now we don't use net specific comment. Use below style:

/*
 * (first line)
 */

(we should apply this style to patch 1/3 as well)

>          * amounts of PCIe AER errors during and after boot up, causing
>          * heavy lags, poor network throughput, and occasional lock-ups.
>          */
> -       if (rtlpriv->rtlhal.hw_type == HARDWARE_TYPE_RTL8723BE &&
> -           (rtlpci->pdev->subsystem_vendor == 0x11ad &&
> -            rtlpci->pdev->subsystem_device == 0x1723)) {
> +       if (pci_match_id(rtl_aspm_quirks, rtlpci->pdev)) {
>                 __rtl_pci_disable_aspm(hw);
>                 ppsc->support_aspm = false;
>         }
> 
> --
> 2.54.0