From: Avraham Stern <redacted>
This version adds the following configuration options:
1. Set the BSS color for NDP ranging
2. Set the minimum and maximum time between measurements for
non trigger based NDP ranging.
3. Terminate the session in case the responder requests LMR feedback.
Signed-off-by: Avraham Stern <redacted>
Signed-off-by: Luca Coelho <redacted>
---
.../wireless/intel/iwlwifi/fw/api/location.h | 119 +++++++++++++++++-
.../wireless/intel/iwlwifi/mvm/constants.h | 5 +-
.../intel/iwlwifi/mvm/ftm-initiator.c | 85 +++++++++++--
3 files changed, 194 insertions(+), 15 deletions(-)
From: Avraham Stern <redacted>
This version adds the following configuration options:
1. Enable/disable setting the session id in the FTM frame
2. Set the BSS color for the responder
3. Set the minimum and maximum time between measurements for
non trigger based NDP ranging.
Signed-off-by: Avraham Stern <redacted>
Signed-off-by: Luca Coelho <redacted>
---
.../wireless/intel/iwlwifi/fw/api/location.h | 70 +++++++++++++++++--
.../intel/iwlwifi/mvm/ftm-responder.c | 27 +++++--
2 files changed, 88 insertions(+), 9 deletions(-)
@@ -1,7 +1,7 @@// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause/**Copyright(C)2015-2017IntelDeutschlandGmbH-*Copyright(C)2018-2020IntelCorporation+*Copyright(C)2018-2021IntelCorporation*/#include<net/cfg80211.h>#include<linux/etherdevice.h>
@@ -77,7 +77,7 @@ static int iwl_mvm_ftm_responder_set_bw_v2(struct cfg80211_chan_def *chandef,staticvoidiwl_mvm_ftm_responder_set_ndp(structiwl_mvm*mvm,-structiwl_tof_responder_config_cmd_v8*cmd)+structiwl_tof_responder_config_cmd_v9*cmd){/* Up to 2 R2I STS are allowed on the responder */u32r2i_max_sts=IWL_MVM_FTM_R2I_MAX_STS<2?
@@ -115,10 +115,27 @@ iwl_mvm_ftm_responder_cmd(struct iwl_mvm *mvm,u8cmd_ver=iwl_fw_lookup_cmd_ver(mvm->fw,LOCATION_GROUP,TOF_RESPONDER_CONFIG_CMD,6);interr;+intcmd_size;lockdep_assert_held(&mvm->mutex);-if(cmd_ver==8)+/* Use a default of bss_color=1 for now */+if(cmd_ver==9){+cmd.cmd_valid_fields|=+cpu_to_le32(IWL_TOF_RESPONDER_CMD_VALID_BSS_COLOR|+IWL_TOF_RESPONDER_CMD_VALID_MIN_MAX_TIME_BETWEEN_MSR);+cmd.bss_color=1;+cmd.min_time_between_msr=+cpu_to_le16(IWL_MVM_FTM_NON_TB_MIN_TIME_BETWEEN_MSR);+cmd.max_time_between_msr=+cpu_to_le16(IWL_MVM_FTM_NON_TB_MAX_TIME_BETWEEN_MSR);+cmd_size=sizeof(structiwl_tof_responder_config_cmd_v9);+}else{+/* All versions up to version 8 have the same size */+cmd_size=sizeof(structiwl_tof_responder_config_cmd_v8);+}++if(cmd_ver>=8)iwl_mvm_ftm_responder_set_ndp(mvm,&cmd);if(cmd_ver>=7)
@@ -137,7 +154,7 @@ if (cmd_ver == 8)returniwl_mvm_send_cmd_pdu(mvm,iwl_cmd_id(TOF_RESPONDER_CONFIG_CMD,LOCATION_GROUP,0),-0,sizeof(cmd),&cmd);+0,cmd_size,&cmd);}staticint
From: Dror Moshe <redacted>
Move code that generates the pnvm file name to a separate function,
so that it can be reused.
Signed-off-by: Dror Moshe <redacted>
Signed-off-by: Luca Coelho <redacted>
---
drivers/net/wireless/intel/iwlwifi/fw/pnvm.c | 13 ++-----------
drivers/net/wireless/intel/iwlwifi/fw/pnvm.h | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 11 deletions(-)
From: Gregory Greenman <redacted>
The new version sends station id in the notification. It's still not
used, but need to adjust the code since the offset of the data was
changed.
Signed-off-by: Gregory Greenman <redacted>
Signed-off-by: Luca Coelho <redacted>
---
.../wireless/intel/iwlwifi/fw/api/offload.h | 31 +++++++++++++++++--
.../net/wireless/intel/iwlwifi/mvm/mac-ctxt.c | 26 ++++++++++++++--
drivers/net/wireless/intel/iwlwifi/mvm/ops.c | 2 +-
3 files changed, 52 insertions(+), 7 deletions(-)
@@ -1460,7 +1480,7 @@ void iwl_mvm_rx_stored_beacon_notif(struct iwl_mvm *mvm,rx_status.band);/* copy the data */-skb_put_data(skb,sb->data,size);+skb_put_data(skb,data,size);memcpy(IEEE80211_SKB_RXCB(skb),&rx_status,sizeof(rx_status));/* pass it as regular rx to mac80211 */
From: Shaul Triebitz <redacted>
Tell the firmware about broadcast TWT support
even if individual TWT is not supported.
In that case the firmware will negotiate only
a broadcast TWT session.
Signed-off-by: Shaul Triebitz <redacted>
Signed-off-by: Luca Coelho <redacted>
---
drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
From: Ilan Peer <redacted>
The iwl_mvm_scan_ch_n_aps_flag() is called with a variable
before the value of the variable is set. Fix it.
Signed-off-by: Ilan Peer <redacted>
Signed-off-by: Luca Coelho <redacted>
---
drivers/net/wireless/intel/iwlwifi/mvm/scan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Johannes Berg <redacted>
To check beacon protection had a mismatch of the key ID we
currently use the key ID from the firmware, but firmware
side we want to clean up the API to stop reporting this.
Instead, check the IWL_RX_MPDU_STATUS_KEY_VALID bit that
indicates that the firmware used the correct key, and if
that's set but we get invalid MIC/replay use the key ID
from the frame to look up the key and notify mac80211 of
MIC error or replay. Since both keys must have the same
cipher and thus MIC length, we can use either of them to
look up the MIC length.
Signed-off-by: Johannes Berg <redacted>
Signed-off-by: Luca Coelho <redacted>
---
drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c | 41 ++++++++++++-------
1 file changed, 27 insertions(+), 14 deletions(-)
@@ -279,7 +279,6 @@ static int iwl_mvm_rx_mgmt_prot(struct ieee80211_sta *sta,{structiwl_mvm_sta*mvmsta;structiwl_mvm_vif*mvmvif;-u8fwkeyid=u32_get_bits(status,IWL_RX_MPDU_STATUS_KEY);u8keyid;structieee80211_key_conf*key;u32len=le16_to_cpu(desc->mpdu_len);
@@ -299,6 +298,10 @@ static int iwl_mvm_rx_mgmt_prot(struct ieee80211_sta *sta,if(!ieee80211_is_beacon(hdr->frame_control))return0;+/* key mismatch - will also report !MIC_OK but we shouldn't count it */+if(!(status&IWL_RX_MPDU_STATUS_KEY_VALID))+return-1;+/* good cases */if(likely(status&IWL_RX_MPDU_STATUS_MIC_OK&&!(status&IWL_RX_MPDU_STATUS_REPLAY_ERROR)))
@@ -309,26 +312,36 @@ static int iwl_mvm_rx_mgmt_prot(struct ieee80211_sta *sta,mvmsta=iwl_mvm_sta_from_mac80211(sta);-/* what? */-if(fwkeyid!=6&&fwkeyid!=7)-return-1;-mvmvif=iwl_mvm_vif_from_mac80211(mvmsta->vif);-key=rcu_dereference(mvmvif->bcn_prot.keys[fwkeyid-6]);-if(!key)-return-1;+/*+*bothkeyswillhavethesamecipherandMIClength,use+*whicheveroneisavailable+*/+key=rcu_dereference(mvmvif->bcn_prot.keys[0]);+if(!key){+key=rcu_dereference(mvmvif->bcn_prot.keys[1]);+if(!key)+return-1;+}if(len<key->icv_len+IEEE80211_GMAC_PN_LEN+2)return-1;-/*-*SeeifthekeyIDmatches-ifnotthismaybeduetoa-*switchandthefirmwaremayerroneouslyreport!MIC_OK.-*/+/* get the real key ID */keyid=frame[len-key->icv_len-IEEE80211_GMAC_PN_LEN-2];-if(keyid!=fwkeyid)-return-1;+/* and if that's the other key, look it up */+if(keyid!=key->keyidx){+/*+*shouldn'thappensincefirmwarechecked,butbesafe+*incasetheMIClengthiswrongtoo,forexample+*/+if(keyid!=6&&keyid!=7)+return-1;+key=rcu_dereference(mvmvif->bcn_prot.keys[keyid-6]);+if(!key)+return-1;+}/* Report status to mac80211 */if(!(status&IWL_RX_MPDU_STATUS_MIC_OK))
From: Matti Gottlieb <redacted>
When having a blank OTP the only way to get the rf id
and the cdb info is from prph registers.
Currently there is some implementation for this, but it
is located in the wrong place in the code (should be before
trying to understand what HW is connected and not after),
and it has a partial implementation.
Signed-off-by: Matti Gottlieb <redacted>
Signed-off-by: Luca Coelho <redacted>
---
drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 2 +-
.../wireless/intel/iwlwifi/fw/error-dump.h | 4 -
drivers/net/wireless/intel/iwlwifi/iwl-prph.h | 38 ++++++++
drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 97 +++++++++++++++++++
4 files changed, 136 insertions(+), 5 deletions(-)
@@ -342,10 +342,6 @@ struct iwl_fw_ini_dump_cfg_name {#define IWL_AX210_HW_TYPE 0x42/* How many bits to roll when adding to the HW type of AX210 HW */#define IWL_AX210_HW_TYPE_ADDITION_SHIFT 12-/* This prph is used to tell apart HW_TYPE == 0x42 NICs */-#define WFPM_OTP_CFG1_ADDR 0xd03098-#define WFPM_OTP_CFG1_IS_JACKET_BIT BIT(4)-#define WFPM_OTP_CFG1_IS_CDB_BIT BIT(5)/* struct iwl_fw_ini_dump_info - ini dump information*@version:dumpversion
@@ -1120,6 +1120,93 @@ static const struct iwl_dev_info iwl_dev_info_table[] = {#endif /* CONFIG_IWLMVM */};+/*+*IncasethatthereisnoOTPontheNIC,gettherfidandcdbinfo+*fromtheprphregisters.+*/+staticintget_crf_id(structiwl_trans*iwl_trans)+{+intret=0;+u32wfpm_ctrl_addr;+u32wfpm_otp_cfg_addr;+u32sd_reg_ver_addr;+u32cdb=0;+structiwl_crf_chip_id_regreg={0};++if(iwl_trans->trans_cfg->device_family>=IWL_DEVICE_FAMILY_AX210){+wfpm_ctrl_addr=WFPM_CTRL_REG_GEN2;+wfpm_otp_cfg_addr=WFPM_OTP_CFG1_ADDR_GEN2;+sd_reg_ver_addr=SD_REG_VER_GEN2;+/* Qu/Pu families have other addresses */+}else{+wfpm_ctrl_addr=WFPM_CTRL_REG;+wfpm_otp_cfg_addr=WFPM_OTP_CFG1_ADDR;+sd_reg_ver_addr=SD_REG_VER;+}++if(iwl_trans_grab_nic_access(iwl_trans)){+u32val;++/* Enable access to peripheral registers */+val=iwl_read_umac_prph_no_grab(iwl_trans,wfpm_ctrl_addr);+val|=ENABLE_WFPM;+iwl_write_umac_prph_no_grab(iwl_trans,wfpm_ctrl_addr,val);++/* Read crf info */+val=iwl_read_prph_no_grab(iwl_trans,sd_reg_ver_addr);+memcpy(®,&val,sizeof(reg));++/* Read cdb info (also contains the jacket info if needed in the future */+cdb=iwl_read_umac_prph_no_grab(iwl_trans,wfpm_otp_cfg_addr);++/* Map between crf id to rf id */+switch(reg.type){+caseREG_CRF_ID_TYPE_JF_1:+iwl_trans->hw_rf_id=(IWL_CFG_RF_TYPE_JF1<<12);+break;+caseREG_CRF_ID_TYPE_JF_2:+iwl_trans->hw_rf_id=(IWL_CFG_RF_TYPE_JF2<<12);+break;+caseREG_CRF_ID_TYPE_HR_NONE_CDB:+iwl_trans->hw_rf_id=(IWL_CFG_RF_TYPE_HR1<<12);+break;+caseREG_CRF_ID_TYPE_HR_CDB:+iwl_trans->hw_rf_id=(IWL_CFG_RF_TYPE_HR2<<12);+break;+caseREG_CRF_ID_TYPE_GF:+iwl_trans->hw_rf_id=(IWL_CFG_RF_TYPE_GF<<12);+break;+caseREG_CRF_ID_TYPE_MR:+iwl_trans->hw_rf_id=(IWL_CFG_RF_TYPE_MR<<12);+break;+default:+ret=-EIO;+IWL_ERR(iwl_trans,+"Can find a correct rfid for crf id 0x%x\n",+reg.type);+iwl_trans_release_nic_access(iwl_trans);+gotoout_get_crf_id;+}+iwl_trans_release_nic_access(iwl_trans);+}else{+IWL_ERR(iwl_trans,"Failed to grab nic access before reading crf id\n");+ret=-EIO;+gotoout_get_crf_id;+}++/* Set CDB capabilities */+if(cdb&BIT(4)){+iwl_trans->hw_rf_id+=BIT(28);+IWL_INFO(iwl_trans,"Adding cdb to rf id\n");+}++IWL_INFO(iwl_trans,"Detected RF 0x%x from crf id 0x%x\n",+iwl_trans->hw_rf_id,reg.type);++out_get_crf_id:+returnret;+}+/* PCI registers */#define PCI_CFG_RETRY_TIMEOUT 0x041
From: Matti Gottlieb <redacted>
Start scratch debug register for Bz family.
This register is used for FW debug, and the driver
should start this register with a fixed value, during
init, and upon an error, should read it, and add it to
the dump.
Signed-off-by: Matti Gottlieb <redacted>
Signed-off-by: Luca Coelho <redacted>
---
drivers/net/wireless/intel/iwlwifi/fw/dump.c | 7 +++++++
drivers/net/wireless/intel/iwlwifi/iwl-csr.h | 7 +++++++
drivers/net/wireless/intel/iwlwifi/pcie/trans-gen2.c | 8 +++++---
3 files changed, 19 insertions(+), 3 deletions(-)
@@ -466,13 +466,15 @@ int iwl_trans_pcie_gen2_start_fw(struct iwl_trans *trans,iwl_pcie_set_ltr(trans);-if(trans->trans_cfg->device_family>=IWL_DEVICE_FAMILY_BZ)+if(trans->trans_cfg->device_family>=IWL_DEVICE_FAMILY_BZ){+iwl_write32(trans,CSR_FUNC_SCRATCH,CSR_FUNC_SCRATCH_INIT_VALUE);iwl_set_bit(trans,CSR_GP_CNTRL,CSR_GP_CNTRL_REG_FLAG_ROM_START);-elseif(trans->trans_cfg->device_family>=IWL_DEVICE_FAMILY_AX210)+}elseif(trans->trans_cfg->device_family>=IWL_DEVICE_FAMILY_AX210){iwl_write_umac_prph(trans,UREG_CPU_INIT_RUN,1);-else+}else{iwl_write_prph(trans,UREG_CPU_INIT_RUN,1);+}/* re-check RF-Kill state since we may have missed the interrupt */hw_rfkill=iwl_pcie_check_hw_rf_kill(trans);
From: Matti Gottlieb <redacted>
Add support for GA and for BZ with FM rf.
Signed-off-by: Matti Gottlieb <redacted>
Signed-off-by: Luca Coelho <redacted>
---
.../net/wireless/intel/iwlwifi/cfg/22000.c | 23 +++++++++++++++++++
.../net/wireless/intel/iwlwifi/iwl-config.h | 3 +++
drivers/net/wireless/intel/iwlwifi/iwl-prph.h | 1 +
drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 14 +++++++++++
4 files changed, 41 insertions(+)
From: Kalle Valo <hidden> Date: 2021-08-21 14:01:56
Luca Coelho [off-list ref] writes:
From: Avraham Stern <redacted>
This version adds the following configuration options:
1. Enable/disable setting the session id in the FTM frame
2. Set the BSS color for the responder
3. Set the minimum and maximum time between measurements for
non trigger based NDP ranging.
Signed-off-by: Avraham Stern <redacted>
Signed-off-by: Luca Coelho <redacted>
From: Kalle Valo <hidden> Date: 2021-08-21 14:04:58
Luca Coelho [off-list ref] writes:
From: Luca Coelho <redacted>
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 <redacted>
From: Kalle Valo <hidden> Date: 2021-08-21 14:08:22
Luca Coelho [off-list ref] writes:
From: Matti Gottlieb <redacted>
When having a blank OTP the only way to get the rf id
and the cdb info is from prph registers.
Currently there is some implementation for this, but it
is located in the wrong place in the code (should be before
trying to understand what HW is connected and not after),
and it has a partial implementation.
Signed-off-by: Matti Gottlieb <redacted>
Signed-off-by: Luca Coelho <redacted>
On Sat, 2021-08-21 at 17:01 +0300, Kalle Valo wrote:
Luca Coelho [off-list ref] writes:
quoted
From: Avraham Stern <redacted>
This version adds the following configuration options:
1. Enable/disable setting the session id in the FTM frame
2. Set the BSS color for the responder
3. Set the minimum and maximum time between measurements for
non trigger based NDP ranging.
Signed-off-by: Avraham Stern <redacted>
Signed-off-by: Luca Coelho <redacted>
On Sat, 2021-08-21 at 17:04 +0300, Kalle Valo wrote:
Luca Coelho [off-list ref] writes:
quoted
From: Luca Coelho <redacted>
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 <redacted>
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.
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.
Is it okay to keep it?
--
Cheers,
Luca.
On Mon, 2021-08-23 at 11:57 +0300, Luca Coelho wrote:
On Sat, 2021-08-21 at 17:04 +0300, Kalle Valo wrote:
quoted
Luca Coelho [off-list ref] writes:
quoted
From: Luca Coelho <redacted>
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 <redacted>
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.
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.
Is it okay to keep it?
Johannes suggested to add "if EXPERT" here, so I'm going to do that and
resend.
--
Cheers,
Luca.
On Sat, 2021-08-21 at 17:07 +0300, Kalle Valo wrote:
Luca Coelho [off-list ref] writes:
quoted
From: Matti Gottlieb <redacted>
When having a blank OTP the only way to get the rf id
and the cdb info is from prph registers.
Currently there is some implementation for this, but it
is located in the wrong place in the code (should be before
trying to understand what HW is connected and not after),
and it has a partial implementation.
Signed-off-by: Matti Gottlieb <redacted>
Signed-off-by: Luca Coelho <redacted>
It's not exactly that this is not endian safe, but we had two issues:
1. AFAIK these bitfields are not guaranteed to be kept in order, so we
shouldn't use them. I'll change it to decode this in some other way.
2. We are actually reading the register without caring for endianess.
I will fix it.
Expect v2 soon.
--
Cheers,
Luca.
quoted
+ /* Set CDB capabilities */
+ if (cdb & BIT(4)) {
+ iwl_trans->hw_rf_id += BIT(28);
+ IWL_INFO(iwl_trans, "Adding cdb to rf id\n");
+ }
On Thu, 2021-08-26 at 14:30 +0300, Luca Coelho wrote:
On Sat, 2021-08-21 at 17:07 +0300, Kalle Valo wrote:
quoted
Luca Coelho [off-list ref] writes:
quoted
From: Matti Gottlieb <redacted>
When having a blank OTP the only way to get the rf id
and the cdb info is from prph registers.
Currently there is some implementation for this, but it
is located in the wrong place in the code (should be before
trying to understand what HW is connected and not after),
and it has a partial implementation.
Signed-off-by: Matti Gottlieb <redacted>
Signed-off-by: Luca Coelho <redacted>
It's not exactly that this is not endian safe, but we had two issues:
1. AFAIK these bitfields are not guaranteed to be kept in order, so we
shouldn't use them. I'll change it to decode this in some other way.
2. We are actually reading the register without caring for endianess.
I will fix it.
Oops, as Johannes pointed out offline, this is not an issue, actually.
I got confused with some places where we do cpu_to_le32() after
reading, which is the opposite and essentially proves that the read is
in cpu-endianess. So I won't "fix" it. ;)
--
Cheers,
Luca.
From: Kalle Valo <hidden> Date: 2021-08-29 11:28:21
Luca Coelho [off-list ref] writes:
On Thu, 2021-08-26 at 14:30 +0300, Luca Coelho wrote:
quoted
On Sat, 2021-08-21 at 17:07 +0300, Kalle Valo wrote:
quoted
Luca Coelho [off-list ref] writes:
quoted
From: Matti Gottlieb <redacted>
When having a blank OTP the only way to get the rf id
and the cdb info is from prph registers.
Currently there is some implementation for this, but it
is located in the wrong place in the code (should be before
trying to understand what HW is connected and not after),
and it has a partial implementation.
Signed-off-by: Matti Gottlieb <redacted>
Signed-off-by: Luca Coelho <redacted>
It's not exactly that this is not endian safe, but we had two issues:
1. AFAIK these bitfields are not guaranteed to be kept in order, so we
shouldn't use them. I'll change it to decode this in some other way.
2. We are actually reading the register without caring for endianess.
I will fix it.
Oops, as Johannes pointed out offline, this is not an issue, actually.
I got confused with some places where we do cpu_to_le32() after
reading, which is the opposite and essentially proves that the read is
in cpu-endianess. So I won't "fix" it. ;)
From: Kalle Valo <hidden> Date: 2021-10-18 07:51:09
Luca Coelho [off-list ref] writes:
On Sat, 2021-08-21 at 17:04 +0300, Kalle Valo wrote:
quoted
Luca Coelho [off-list ref] writes:
quoted
From: Luca Coelho <redacted>
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 <redacted>
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.
On Mon, 2021-10-18 at 10:51 +0300, Kalle Valo wrote:
Luca Coelho [off-list ref] writes:
quoted
On Sat, 2021-08-21 at 17:04 +0300, Kalle Valo wrote:
quoted
Luca Coelho [off-list ref] writes:
quoted
From: Luca Coelho <redacted>
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 <redacted>
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?
quoted
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.
quoted
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.
Okay, fair enough. I'll hold this back now, rework it without the
Kconfig option and send it again in the future.
Thanks for the comments!
--
Cheers,
Luca.