RE: [PATCH v4 08/11] wifi: rtw88: Add rtw8821cu chipset support

[Ping-Ke Shih]

> -----Original Message-----
> From: Kalle Valo <kvalo@xxxxxxxxxx>
> Sent: Thursday, December 8, 2022 10:21 PM
> To: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
> Cc: Jakub Kicinski <kuba@xxxxxxxxxx>; Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>;
> linux-wireless@xxxxxxxxxxxxxxx; Neo Jou <neojou@xxxxxxxxx>; Hans Ulli Kroll <linux@xxxxxxxxxxxxx>; Ping-Ke
> Shih <pkshih@xxxxxxxxxxx>; Yan-Hsuan Chuang <tony0620emma@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>;
> kernel@xxxxxxxxxxxxxx; Johannes Berg <johannes@xxxxxxxxxxxxxxxx>; Alexander Hochbaum <alex@xxxxxxxxxx>;
> Da Xue <da@libre.computer>; Bernie Huang <phhuang@xxxxxxxxxxx>; Viktor Petrenko <g0000ga@xxxxxxxxx>
> Subject: Re: [PATCH v4 08/11] wifi: rtw88: Add rtw8821cu chipset support
> 
> Larry Finger <Larry.Finger@xxxxxxxxxxxx> 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