Luca Coelho <luca@xxxxxxxxx> writes: > On Sat, 2021-08-21 at 17:04 +0300, Kalle Valo wrote: >> Luca Coelho <luca@xxxxxxxxx> writes: >> >> > From: Luca Coelho <luciano.coelho@xxxxxxxxx> >> > >> > Export the debug host command framework and add the twt_setup entry. >> > This will allow external parties to use these debugging features. >> > More entries can be added later on. >> > >> > Signed-off-by: Luca Coelho <luciano.coelho@xxxxxxxxx> >> >> [...] >> >> > --- a/drivers/net/wireless/intel/iwlwifi/Kconfig >> > +++ b/drivers/net/wireless/intel/iwlwifi/Kconfig >> > @@ -92,6 +92,12 @@ config IWLWIFI_BCAST_FILTERING >> > If unsure, don't enable this option, as some programs might >> > expect incoming broadcasts for their normal operations. >> > >> > >> > +config IWLWIFI_DHC >> > + bool "Enable debug host commands" >> > + help >> > + This option enables the debug host command API. It's used >> > + for debugging and validation purposes. >> > + >> >> Why a new Kconfig option? Those should not be added lightly. > > This is a debugging feature that is not really needed in production > kernels, so we prefer to allow it to be removed so we don't waste > resources. What resources exactly? I would say if the admin or distro maintainer wants to save on resources he will disable IWLWIFI_DEBUGFS. Why do we need to have multiple Kconfig options for iwlwifi debugfs interface? > We're publishing this for a few reasons: > > 1. it will help prevent rebasing mistakes when sending patches upstream > from our internal tree, because a lot of this code is spread around the > driver; > > 2. in some occasions, we may ask advanced users to enable it so we can > get more data and run more tests in case of tricky bugs; > > 3. for the specific case of twt_setup, this allows running some TWT > test scenarios with our driver that wouldn't be easily available > otherwise. Sure, I understand all that. The better debug features we have in upstream the better. But I don't understand why a new Kconfig option is needed for DHC feature. > Is it okay to keep it? In the past Linus has stated his dislike of adding pointless Kconfig options, with which I strongly agree, and to me it looks like IWLWIFI_DHC is exactly that. So I'm very hesitant about this. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
