From: Kalle Valo <kvalo@codeaurora.org>
To: Luca Coelho <luca@coelho.fi>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH 08/12] iwlwifi: export DHC framework and add first public entry, twt_setup
Date: Mon, 18 Oct 2021 10:51:01 +0300 [thread overview]
Message-ID: <877deawz2y.fsf@codeaurora.org> (raw)
In-Reply-To: <3be8a0e1cbe82e0c4b55b00c7e7fe06d8014aa71.camel@coelho.fi> (Luca Coelho's message of "Mon, 23 Aug 2021 11:57:45 +0300")
Luca Coelho <luca@coelho.fi> writes:
> On Sat, 2021-08-21 at 17:04 +0300, Kalle Valo wrote:
>> Luca Coelho <luca@coelho.fi> writes:
>>
>> > From: Luca Coelho <luciano.coelho@intel.com>
>> >
>> > 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@intel.com>
>>
>> [...]
>>
>> > --- 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
next prev parent reply other threads:[~2021-10-18 7:51 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-20 11:03 [PATCH 00/13] iwlwifi: updates intended for v5.15 2021-08-20 Luca Coelho
2021-08-20 11:03 ` [PATCH 01/12] iwlwifi: mvm: add support for range request command version 13 Luca Coelho
2021-08-20 11:03 ` [PATCH 02/12] iwlwifi: mvm: add support for resonder config command version 9 Luca Coelho
2021-08-21 14:01 ` Kalle Valo
2021-08-23 8:51 ` Luca Coelho
2021-08-20 11:03 ` [PATCH 03/12] iwlwifi: move get pnvm file name to a separate function Luca Coelho
2021-08-20 11:03 ` [PATCH 04/12] iwlwifi: mvm: introduce iwl_stored_beacon_notif_v3 Luca Coelho
2021-08-20 11:03 ` [PATCH 05/12] iwlwifi: mvm: support broadcast TWT alone Luca Coelho
2021-08-20 11:03 ` [PATCH 06/12] iwlwifi: mvm: Fix scan channel flags settings Luca Coelho
2021-08-20 11:03 ` [PATCH 07/12] iwlwifi: mvm: don't use FW key ID in beacon protection Luca Coelho
2021-08-20 11:03 ` [PATCH 08/12] iwlwifi: export DHC framework and add first public entry, twt_setup Luca Coelho
2021-08-21 14:04 ` Kalle Valo
2021-08-23 8:57 ` Luca Coelho
2021-08-26 11:26 ` Luca Coelho
2021-10-18 7:51 ` Kalle Valo [this message]
2021-10-22 6:28 ` Luca Coelho
2021-08-20 11:03 ` [PATCH 09/12] iwlwifi: mvm: add fixed_rate debugfs entry to public DHC Luca Coelho
2021-08-20 11:03 ` [PATCH 10/12] iwlwifi: Add support for getting rf id with blank otp Luca Coelho
2021-08-21 14:07 ` Kalle Valo
2021-08-26 11:30 ` Luca Coelho
2021-08-26 11:45 ` Luca Coelho
2021-08-29 11:27 ` Kalle Valo
2021-08-20 11:03 ` [PATCH 11/12] iwlwifi: Add support for more BZ HWs Luca Coelho
2021-08-20 11:03 ` [PATCH 12/12] iwlwifi: Start scratch debug register for Bz family Luca Coelho
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877deawz2y.fsf@codeaurora.org \
--to=kvalo@codeaurora.org \
--cc=linux-wireless@vger.kernel.org \
--cc=luca@coelho.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).