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

Kalle Valo
2022-12-08T14:23:31Z

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.

--