| Message ID | 20221129100754.2753237-9-s.hauer@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | RTW88: Add support for USB variants | expand |
On Tue, 29 Nov 2022 11:07:51 +0100 Sascha Hauer wrote: > +config RTW88_8821CU > + tristate "Realtek 8821CU USB wireless network adapter" > + depends on USB > + select RTW88_CORE > + select RTW88_USB > + select RTW88_8821C > + help > + Select this option will enable support for 8821CU chipset > + > + 802.11ac USB wireless network adapter Those kconfig knobs add so little code, why not combine them all into one? No point bothering the user with 4 different questions with amount to almost nothing.
On 11/29/22 10:17, Jakub Kicinski wrote: > On Tue, 29 Nov 2022 11:07:51 +0100 Sascha Hauer wrote: >> +config RTW88_8821CU >> + tristate "Realtek 8821CU USB wireless network adapter" >> + depends on USB >> + select RTW88_CORE >> + select RTW88_USB >> + select RTW88_8821C >> + help >> + Select this option will enable support for 8821CU chipset >> + >> + 802.11ac USB wireless network adapter > > Those kconfig knobs add so little code, why not combine them all into > one? No point bothering the user with 4 different questions with amount > to almost nothing. I see only one knob there, name RTW88_8821CU. The other configuration variables select parts of the code that are shared with other drivers such as RTW88_8821CE and these parts must be there. Larry
On Tue, Nov 29, 2022 at 08:17:53AM -0800, Jakub Kicinski wrote: > On Tue, 29 Nov 2022 11:07:51 +0100 Sascha Hauer wrote: > > +config RTW88_8821CU > > + tristate "Realtek 8821CU USB wireless network adapter" > > + depends on USB > > + select RTW88_CORE > > + select RTW88_USB > > + select RTW88_8821C > > + help > > + Select this option will enable support for 8821CU chipset > > + > > + 802.11ac USB wireless network adapter > > Those kconfig knobs add so little code, why not combine them all into > one? No point bothering the user with 4 different questions with amount > to almost nothing. I tend to agree here. I followed the pattern used with PCI support here, but I also think that we don't need to be able to select all chips individually. The following should be enough: config RTW88_PCI tristate depends on PCI default y config RTW88_USB tristate depends on USB default y Still I'd like to continue with the current pattern to not block merging of the USB support with this topic. I could create a follow up patch though if that's desired. Sascha
Sascha Hauer <s.hauer@pengutronix.de> writes: > On Tue, Nov 29, 2022 at 08:17:53AM -0800, Jakub Kicinski wrote: >> On Tue, 29 Nov 2022 11:07:51 +0100 Sascha Hauer wrote: >> > +config RTW88_8821CU >> > + tristate "Realtek 8821CU USB wireless network adapter" >> > + depends on USB >> > + select RTW88_CORE >> > + select RTW88_USB >> > + select RTW88_8821C >> > + help >> > + Select this option will enable support for 8821CU chipset >> > + >> > + 802.11ac USB wireless network adapter >> >> Those kconfig knobs add so little code, why not combine them all into >> one? No point bothering the user with 4 different questions with amount >> to almost nothing. > > I tend to agree here. I followed the pattern used with PCI support here, > but I also think that we don't need to be able to select all chips > individually. The following should be enough: > > config RTW88_PCI > tristate > depends on PCI > default y > > config RTW88_USB > tristate > depends on USB > default y > > Still I'd like to continue with the current pattern to not block merging > of the USB support with this topic. > > I could create a follow up patch though if that's desired. Yeah, a follow up patch is a good idea. Best to get USB support commited first, after that we can discuss improvements.
Larry Finger <Larry.Finger@lwfinger.net> writes: > On 11/29/22 10:17, Jakub Kicinski wrote: >> On Tue, 29 Nov 2022 11:07:51 +0100 Sascha Hauer wrote: >>> +config RTW88_8821CU >>> + tristate "Realtek 8821CU USB wireless network adapter" >>> + depends on USB >>> + select RTW88_CORE >>> + select RTW88_USB >>> + select RTW88_8821C >>> + help >>> + Select this option will enable support for 8821CU chipset >>> + >>> + 802.11ac USB wireless network adapter >> >> Those kconfig knobs add so little code, why not combine them all into >> one? No point bothering the user with 4 different questions with amount >> to almost nothing. > > I see only one knob there, name RTW88_8821CU. The other configuration > variables select parts of the code that are shared with other drivers > such as RTW88_8821CE and these parts must be there. I just test compiled these patches and we have four new questions: Realtek 8822BU USB wireless network adapter (RTW88_8822BU) [N/m/?] (NEW) m Realtek 8822CU USB wireless network adapter (RTW88_8822CU) [N/m/?] (NEW) m Realtek 8723DU USB wireless network adapter (RTW88_8723DU) [N/m/?] (NEW) m Realtek 8821CU USB wireless network adapter (RTW88_8821CU) [N/m/?] (NEW) To me this looks too fine grained. Does it really make sense, for example, to enable RTW88_8822BU but not RTW88_8822CU? Would just having RTW88_USB containing all USB devices be more sensible? And the same for PCI, and if we have in the future, SDIO devices. But like discussed earlier, to keep things simple let's handle that separately from this patchset.
> -----Original Message----- > From: Kalle Valo <kvalo@kernel.org> > Sent: Thursday, December 8, 2022 10:21 PM > To: Larry Finger <Larry.Finger@lwfinger.net> > Cc: Jakub Kicinski <kuba@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>; > linux-wireless@vger.kernel.org; Neo Jou <neojou@gmail.com>; Hans Ulli Kroll <linux@ulli-kroll.de>; Ping-Ke > Shih <pkshih@realtek.com>; Yan-Hsuan Chuang <tony0620emma@gmail.com>; netdev@vger.kernel.org; > linux-kernel@vger.kernel.org; Martin Blumenstingl <martin.blumenstingl@googlemail.com>; > kernel@pengutronix.de; Johannes Berg <johannes@sipsolutions.net>; Alexander Hochbaum <alex@appudo.com>; > Da Xue <da@libre.computer>; Bernie Huang <phhuang@realtek.com>; Viktor Petrenko <g0000ga@gmail.com> > Subject: Re: [PATCH v4 08/11] wifi: rtw88: Add rtw8821cu chipset support > > Larry Finger <Larry.Finger@lwfinger.net> writes: > > > On 11/29/22 10:17, Jakub Kicinski wrote: > >> On Tue, 29 Nov 2022 11:07:51 +0100 Sascha Hauer wrote: > >>> +config RTW88_8821CU > >>> + tristate "Realtek 8821CU USB wireless network adapter" > >>> + depends on USB > >>> + select RTW88_CORE > >>> + select RTW88_USB > >>> + select RTW88_8821C > >>> + help > >>> + Select this option will enable support for 8821CU chipset > >>> + > >>> + 802.11ac USB wireless network adapter > >> > >> Those kconfig knobs add so little code, why not combine them all into > >> one? No point bothering the user with 4 different questions with amount > >> to almost nothing. > > > > I see only one knob there, name RTW88_8821CU. The other configuration > > variables select parts of the code that are shared with other drivers > > such as RTW88_8821CE and these parts must be there. > > I just test compiled these patches and we have four new questions: > > Realtek 8822BU USB wireless network adapter (RTW88_8822BU) [N/m/?] (NEW) m > Realtek 8822CU USB wireless network adapter (RTW88_8822CU) [N/m/?] (NEW) m > Realtek 8723DU USB wireless network adapter (RTW88_8723DU) [N/m/?] (NEW) m > Realtek 8821CU USB wireless network adapter (RTW88_8821CU) [N/m/?] (NEW) > > To me this looks too fine grained. Does it really make sense, for > example, to enable RTW88_8822BU but not RTW88_8822CU? Would just having > RTW88_USB containing all USB devices be more sensible? And the same for > PCI, and if we have in the future, SDIO devices. > Summerize Realtek 802.11n/11ac WiFi drivers after this patchset: Kconfig driver #-of-ko knob support chips --------------------------------------------------------------------- rtl8xxxu 1 1 8188fu, 8192cu, 8192eu, 8723au, 8723bu rtlwifi 15 9 8192se, 8723ae, 8723be, 8192ee, 8192de, 8188ee, 8192ce, 8821ae 8192cu rtw88 15 8 8723de, 8821ce, 8822be, 8822ce 8723du, 8821cu, 8822bu, 8822cu If we merge into single one Kconfig knob, we could have a long list name "Realtek 8723DU/8821CU/8822BU/8822CU USB wireless network adapter" or an implicit name "Realtek 802.11n/802.11ac USB wireless network adapter" The string mixes "802.11n/802.11ac" because hardware architecture of Realtek WiFi chips change during 11n/11ac generations, so rtlwifi (old architecture) and rtw88 (new architecture) support both 11n and 11ac chips. That is a little bit inconvenient to people who wants to know which driver support his own WiFi module explicitly. Another thing is to save some compile time and disk space to build these .ko if we have separated knobs. For Ubuntu or other distros users, I think they may not care about this, because distros have already built drivers and disk of notebook or desktop is large. But, for embedded users, like Raspberry Pi or proprietary embedded system, they may want to highly customize drivers due to limit of hardware resource. Therefore, I prefer to preserve current Kconfig. Though single one knob is really simple for anything. Ping-Ke
diff --git a/drivers/net/wireless/realtek/rtw88/Kconfig b/drivers/net/wireless/realtek/rtw88/Kconfig index 1624c5db69bac..2b500dbefbc2d 100644 --- a/drivers/net/wireless/realtek/rtw88/Kconfig +++ b/drivers/net/wireless/realtek/rtw88/Kconfig @@ -75,6 +75,17 @@ config RTW88_8821CE 802.11ac PCIe wireless network adapter +config RTW88_8821CU + tristate "Realtek 8821CU USB wireless network adapter" + depends on USB + select RTW88_CORE + select RTW88_USB + select RTW88_8821C + help + Select this option will enable support for 8821CU chipset + + 802.11ac USB wireless network adapter + config RTW88_DEBUG bool "Realtek rtw88 debug support" depends on RTW88_CORE diff --git a/drivers/net/wireless/realtek/rtw88/Makefile b/drivers/net/wireless/realtek/rtw88/Makefile index 2c2b0e5133cdf..552661a638def 100644 --- a/drivers/net/wireless/realtek/rtw88/Makefile +++ b/drivers/net/wireless/realtek/rtw88/Makefile @@ -44,6 +44,9 @@ rtw88_8821c-objs := rtw8821c.o rtw8821c_table.o obj-$(CONFIG_RTW88_8821CE) += rtw88_8821ce.o rtw88_8821ce-objs := rtw8821ce.o +obj-$(CONFIG_RTW88_8821CU) += rtw88_8821cu.o +rtw88_8821cu-objs := rtw8821cu.o + obj-$(CONFIG_RTW88_PCI) += rtw88_pci.o rtw88_pci-objs := pci.o diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c b/drivers/net/wireless/realtek/rtw88/rtw8821c.c index 9afdc5ce86b43..17f800f6efbd0 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c @@ -26,6 +26,12 @@ static void rtw8821ce_efuse_parsing(struct rtw_efuse *efuse, ether_addr_copy(efuse->addr, map->e.mac_addr); } +static void rtw8821cu_efuse_parsing(struct rtw_efuse *efuse, + struct rtw8821c_efuse *map) +{ + ether_addr_copy(efuse->addr, map->u.mac_addr); +} + enum rtw8821ce_rf_set { SWITCH_TO_BTG, SWITCH_TO_WLG, @@ -68,6 +74,9 @@ static int rtw8821c_read_efuse(struct rtw_dev *rtwdev, u8 *log_map) case RTW_HCI_TYPE_PCIE: rtw8821ce_efuse_parsing(efuse, map); break; + case RTW_HCI_TYPE_USB: + rtw8821cu_efuse_parsing(efuse, map); + break; default: /* unsupported now */ return -ENOTSUPP; @@ -1148,6 +1157,13 @@ static void rtw8821c_phy_cck_pd_set(struct rtw_dev *rtwdev, u8 new_lvl) dm_info->cck_pd_default + new_lvl * 2); } +static void rtw8821c_fill_txdesc_checksum(struct rtw_dev *rtwdev, + struct rtw_tx_pkt_info *pkt_info, + u8 *txdesc) +{ + fill_txdesc_checksum_common(txdesc, 16); +} + static struct rtw_pwr_seq_cmd trans_carddis_to_cardemu_8821c[] = { {0x0086, RTW_PWR_CUT_ALL_MSK, @@ -1521,6 +1537,7 @@ static const struct rtw_rfe_def rtw8821c_rfe_defs[] = { [2] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), [4] = RTW_DEF_RFE_EXT(8821c, 0, 0, 2), [6] = RTW_DEF_RFE(8821c, 0, 0), + [34] = RTW_DEF_RFE(8821c, 0, 0), }; static struct rtw_hw_reg rtw8821c_dig[] = { @@ -1595,6 +1612,7 @@ static struct rtw_chip_ops rtw8821c_ops = { .config_bfee = rtw8821c_bf_config_bfee, .set_gid_table = rtw_bf_set_gid_table, .cfg_csi_rate = rtw_bf_cfg_csi_rate, + .fill_txdesc_checksum = rtw8821c_fill_txdesc_checksum, .coex_set_init = rtw8821c_coex_cfg_init, .coex_set_ant_switch = rtw8821c_coex_cfg_ant_switch, diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.h b/drivers/net/wireless/realtek/rtw88/rtw8821c.h index 2698801fc35d5..1c81260f3a542 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.h +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.h @@ -9,6 +9,26 @@ #define RCR_VHT_ACK BIT(26) +struct rtw8821cu_efuse { + u8 res4[4]; /* 0xd0 */ + u8 usb_optional_function; + u8 res5[0x1e]; + u8 res6[2]; + u8 serial[0x0b]; /* 0xf5 */ + u8 vid; /* 0x100 */ + u8 res7; + u8 pid; + u8 res8[4]; + u8 mac_addr[ETH_ALEN]; /* 0x107 */ + u8 res9[2]; + u8 vendor_name[0x07]; + u8 res10[2]; + u8 device_name[0x14]; + u8 res11[0xcf]; + u8 package_type; /* 0x1fb */ + u8 res12[0x4]; +}; + struct rtw8821ce_efuse { u8 mac_addr[ETH_ALEN]; /* 0xd0 */ u8 vender_id[2]; @@ -73,6 +93,7 @@ struct rtw8821c_efuse { u8 res[3]; union { struct rtw8821ce_efuse e; + struct rtw8821cu_efuse u; }; }; diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821cu.c b/drivers/net/wireless/realtek/rtw88/rtw8821cu.c new file mode 100644 index 0000000000000..7a5cbdc31ef79 --- /dev/null +++ b/drivers/net/wireless/realtek/rtw88/rtw8821cu.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause +/* Copyright(c) 2018-2019 Realtek Corporation + */ + +#include <linux/module.h> +#include <linux/usb.h> +#include "main.h" +#include "rtw8821c.h" +#include "usb.h" + +static const struct usb_device_id rtw_8821cu_id_table[] = { + { USB_DEVICE_AND_INTERFACE_INFO(RTW_USB_VENDOR_ID_REALTEK, 0xb82b, 0xff, 0xff, 0xff), + .driver_info = (kernel_ulong_t)&(rtw8821c_hw_spec) }, /* 8821CU */ + { USB_DEVICE_AND_INTERFACE_INFO(RTW_USB_VENDOR_ID_REALTEK, 0xb820, 0xff, 0xff, 0xff), + .driver_info = (kernel_ulong_t)&(rtw8821c_hw_spec) }, /* 8821CU */ + { USB_DEVICE_AND_INTERFACE_INFO(RTW_USB_VENDOR_ID_REALTEK, 0xc821, 0xff, 0xff, 0xff), + .driver_info = (kernel_ulong_t)&(rtw8821c_hw_spec) }, /* 8821CU */ + { USB_DEVICE_AND_INTERFACE_INFO(RTW_USB_VENDOR_ID_REALTEK, 0xc820, 0xff, 0xff, 0xff), + .driver_info = (kernel_ulong_t)&(rtw8821c_hw_spec) }, /* 8821CU */ + { USB_DEVICE_AND_INTERFACE_INFO(RTW_USB_VENDOR_ID_REALTEK, 0xc82a, 0xff, 0xff, 0xff), + .driver_info = (kernel_ulong_t)&(rtw8821c_hw_spec) }, /* 8821CU */ + { USB_DEVICE_AND_INTERFACE_INFO(RTW_USB_VENDOR_ID_REALTEK, 0xc82b, 0xff, 0xff, 0xff), + .driver_info = (kernel_ulong_t)&(rtw8821c_hw_spec) }, /* 8821CU */ + { USB_DEVICE_AND_INTERFACE_INFO(RTW_USB_VENDOR_ID_REALTEK, 0xc811, 0xff, 0xff, 0xff), + .driver_info = (kernel_ulong_t)&(rtw8821c_hw_spec) }, /* 8811CU */ + { USB_DEVICE_AND_INTERFACE_INFO(RTW_USB_VENDOR_ID_REALTEK, 0x8811, 0xff, 0xff, 0xff), + .driver_info = (kernel_ulong_t)&(rtw8821c_hw_spec) }, /* 8811CU */ + { USB_DEVICE_AND_INTERFACE_INFO(RTW_USB_VENDOR_ID_REALTEK, 0x2006, 0xff, 0xff, 0xff), + .driver_info = (kernel_ulong_t)&(rtw8821c_hw_spec) }, /* TOTOLINK A650UA v3 */ + {}, +}; +MODULE_DEVICE_TABLE(usb, rtw_8821cu_id_table); + +static int rtw_8821cu_probe(struct usb_interface *intf, + const struct usb_device_id *id) +{ + return rtw_usb_probe(intf, id); +} + +static struct usb_driver rtw_8821cu_driver = { + .name = "rtw_8821cu", + .id_table = rtw_8821cu_id_table, + .probe = rtw_8821cu_probe, + .disconnect = rtw_usb_disconnect, +}; +module_usb_driver(rtw_8821cu_driver); + +MODULE_AUTHOR("Hans Ulli Kroll <linux@ulli-kroll.de>"); +MODULE_DESCRIPTION("Realtek 802.11ac wireless 8821cu driver"); +MODULE_LICENSE("Dual BSD/GPL");
Add support for the rtw8821cu chipset based on https://github.com/ulli-kroll/rtw88-usb.git Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- Notes: Changes since v3: - drop unnecessary rtw8821cu.h Changes since v2: - Fix txdesc checksum calculation. The checksum must be calculated over a fixed number of words. drivers/net/wireless/realtek/rtw88/Kconfig | 11 ++++ drivers/net/wireless/realtek/rtw88/Makefile | 3 ++ drivers/net/wireless/realtek/rtw88/rtw8821c.c | 18 +++++++ drivers/net/wireless/realtek/rtw88/rtw8821c.h | 21 ++++++++ .../net/wireless/realtek/rtw88/rtw8821cu.c | 50 +++++++++++++++++++ 5 files changed, 103 insertions(+) create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8821cu.c