There can be cases where the board-2.bin does not contain
any BDF matching the chip-id+board-id+variant combination.
This causes the wlan probe to fail and renders wifi unusable.
For e.g. if the board-2.bin has default BDF as:
bus=snoc,qmi-board-id=67 but for some reason the board-id
on the wlan chip is not programmed and read 0xff as the
default value. In such cases there won't be any matching BDF
because the board-2.bin will be searched with following:
bus=snoc,qmi-board-id=ff
To address these scenarios, there can be an option to provide
fallback default BDF name in the device tree. If none of the
BDF names match then the board-2.bin file can be searched with
default BDF names assigned in the device tree.
The default BDF name can be set as:
wifi@a000000 {
status = "okay";
qcom,ath10k-default-bdf = "bus=snoc,qmi-board-id=67";
};
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2-00696-QCAHLSWMTPL-1
Signed-off-by: Abhishek Kumar <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 30 ++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/core.h | 5 +++++
drivers/net/wireless/ath/ath10k/qmi.c | 4 ++++
3 files changed, 39 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 8f5b8eb368fa..a7008c853f0f 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1081,6 +1081,32 @@ int ath10k_core_check_dt(struct ath10k *ar)
}
EXPORT_SYMBOL(ath10k_core_check_dt);
+int ath10k_core_parse_default_bdf_dt(struct ath10k *ar)
+{
+ struct device_node *node;
+ const char *board_name = NULL;
+
+ ar->id.default_bdf[0] = '\0';
+
+ node = ar->dev->of_node;
+ if (!node)
+ return -ENOENT;
+
+ of_property_read_string(node, "qcom,ath10k-default-bdf",
+ &board_name);
+ if (!board_name)
+ return -ENODATA;
+
+ if (strscpy(ar->id.default_bdf,
+ board_name, sizeof(ar->id.default_bdf)) < 0)
+ ath10k_warn(ar,
+ "default board name is longer than allocated buffer, board_name: %s; allocated size: %d\n",
+ board_name, sizeof(ar->id.default_bdf));
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_parse_default_bdf_dt);
+
static int ath10k_download_fw(struct ath10k *ar)
{
u32 address, data_len;
@@ -1441,6 +1467,10 @@ static int ath10k_core_fetch_board_data_api_n(struct ath10k *ar,
if (ret == -ENOENT && fallback_boardname2)
ret = ath10k_core_search_bd(ar, fallback_boardname2, data, len);
+ /* check default BDF name if provided in device tree */
+ if (ret == -ENOENT && ar->id.default_bdf[0] != '\0')
+ ret = ath10k_core_search_bd(ar, ar->id.default_bdf, data, len);
+
if (ret == -ENOENT) {
ath10k_err(ar,
"failed to fetch board data for %s from %s/%s\n",
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 9f6680b3be0a..1201bb7bb8ab 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -79,6 +79,9 @@
/* The magic used by QCA spec */
#define ATH10K_SMBIOS_BDF_EXT_MAGIC "BDF_"
+/* Default BDF board name buffer size */
+#define ATH10K_DEFAULT_BDF_BUFFER_SIZE 0x40
+
/* Default Airtime weight multipler (Tuned for multiclient performance) */
#define ATH10K_AIRTIME_WEIGHT_MULTIPLIER 4
@@ -1102,6 +1105,7 @@ struct ath10k {
bool ext_bid_supported;
char bdf_ext[ATH10K_SMBIOS_BDF_EXT_STR_LENGTH];
+ char default_bdf[ATH10K_DEFAULT_BDF_BUFFER_SIZE];
} id;
int fw_api;
@@ -1342,6 +1346,7 @@ int ath10k_core_register(struct ath10k *ar,
void ath10k_core_unregister(struct ath10k *ar);
int ath10k_core_fetch_board_file(struct ath10k *ar, int bd_ie_type);
int ath10k_core_check_dt(struct ath10k *ar);
+int ath10k_core_parse_default_bdf_dt(struct ath10k *ar);
void ath10k_core_free_board_files(struct ath10k *ar);
#endif /* _CORE_H_ */
diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c
index 80fcb917fe4e..a57675308014 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -831,6 +831,10 @@ static int ath10k_qmi_fetch_board_file(struct ath10k_qmi *qmi)
if (ret)
ath10k_dbg(ar, ATH10K_DBG_QMI, "DT bdf variant name not set.\n");
+ ret = ath10k_core_parse_default_bdf_dt(ar);
+ if (ret)
+ ath10k_dbg(ar, ATH10K_DBG_QMI, "Default BDF name not set in device tree.\n");
+
return ath10k_core_fetch_board_file(qmi->ar, ATH10K_BD_IE_BOARD);
}
--
2.34.1.575.g55b058a8bb-goog
It is possible that BDF name with board-id+chip-id+variant
combination is not found in the board-2.bin. Such cases can
cause wlan probe to fail and completely break wifi. In such
case there can be an optional property to define a default
BDF name to search for in the board-2.bin file when none of
the combinations (board-id,chip-id,variant) match.
To address the above concern provide an optional proptery:
qcom,ath10k-default-bdf
Signed-off-by: Abhishek Kumar <[email protected]>
---
.../devicetree/bindings/net/wireless/qcom,ath10k.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
index b61c2d5a0ff7..d76d1392863d 100644
--- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
@@ -63,6 +63,10 @@ Optional properties:
hw versions.
- qcom,ath10k-pre-calibration-data : pre calibration data as an array,
the length can vary between hw versions.
+- qcom,ath10k-default-bdf : default board data file name to be searched in
+ board-2.bin. This is searched if no BDF is found
+ in board-2.bin that matches, chip-id, board-id and
+ variant combination
- <supply-name>-supply: handle to the regulator device tree node
optional "supply-name" are "vdd-0.8-cx-mx",
"vdd-1.8-xo", "vdd-1.3-rfa", "vdd-3.3-ch0",
--
2.34.1.575.g55b058a8bb-goog
Hi,
On Fri, Jan 7, 2022 at 12:05 PM Abhishek Kumar <[email protected]> wrote:
>
> There can be cases where the board-2.bin does not contain
> any BDF matching the chip-id+board-id+variant combination.
> This causes the wlan probe to fail and renders wifi unusable.
> For e.g. if the board-2.bin has default BDF as:
> bus=snoc,qmi-board-id=67 but for some reason the board-id
> on the wlan chip is not programmed and read 0xff as the
> default value. In such cases there won't be any matching BDF
> because the board-2.bin will be searched with following:
> bus=snoc,qmi-board-id=ff
> To address these scenarios, there can be an option to provide
> fallback default BDF name in the device tree. If none of the
> BDF names match then the board-2.bin file can be searched with
> default BDF names assigned in the device tree.
>
> The default BDF name can be set as:
> wifi@a000000 {
> status = "okay";
> qcom,ath10k-default-bdf = "bus=snoc,qmi-board-id=67";
Rather than add a new device tree property, wouldn't it be good enough
to leverage the existing variant? Right now I think that the board
file contains:
'bus=snoc,qmi-board-id=67.bin'
'bus=snoc,qmi-board-id=67,qmi-chip-id=320,variant=GO_LAZOR.bin'
'bus=snoc,qmi-board-id=67,qmi-chip-id=320,variant=GO_POMPOM.bin'
'bus=snoc,qmi-board-id=67,qmi-chip-id=4320,variant=GO_LAZOR.bin'
'bus=snoc,qmi-board-id=67,qmi-chip-id=4320,variant=GO_POMPOM.bin'
...and, on lazor for instance, we have:
qcom,ath10k-calibration-variant = "GO_LAZOR";
The problem you're trying to solve is that on some early lazor
prototype hardware we didn't have the "board-id" programmed we could
get back 0xff from the hardware. As I understand it 0xff always means
"unprogrammed".
It feels like you could just have a special case such that if the
hardware reports board ID of 0xff and you don't get a "match" that you
could just treat 0xff as a wildcard. That means that you'd see the
"variant" in the device tree and pick one of the "GO_LAZOR" entries.
Anyway, I guess it's up to the people who spend more time in this file
which they'd prefer, but that seems like it'd be simple and wouldn't
require a bindings addition...
-Doug
Hi Abhishek,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kvalo-ath/ath-next]
[also build test WARNING on kvalo-wireless-drivers-next/master kvalo-wireless-drivers/master v5.16-rc8 next-20220107]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Abhishek-Kumar/ath10k-search-for-default-BDF-name-provided-in-DT/20220108-040711
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git ath-next
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220108/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/98e7f008cb14b9c4038287915c271bb485a89346
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Abhishek-Kumar/ath10k-search-for-default-BDF-name-provided-in-DT/20220108-040711
git checkout 98e7f008cb14b9c4038287915c271bb485a89346
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash drivers/net/wireless/ath/ath10k/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
drivers/net/wireless/ath/ath10k/core.c: In function 'ath10k_core_parse_default_bdf_dt':
>> drivers/net/wireless/ath/ath10k/core.c:1103:115: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long unsigned int' [-Wformat=]
1103 | "default board name is longer than allocated buffer, board_name: %s; allocated size: %d\n",
| ~^
| |
| int
| %ld
1104 | board_name, sizeof(ar->id.default_bdf));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| long unsigned int
vim +1103 drivers/net/wireless/ath/ath10k/core.c
1083
1084 int ath10k_core_parse_default_bdf_dt(struct ath10k *ar)
1085 {
1086 struct device_node *node;
1087 const char *board_name = NULL;
1088
1089 ar->id.default_bdf[0] = '\0';
1090
1091 node = ar->dev->of_node;
1092 if (!node)
1093 return -ENOENT;
1094
1095 of_property_read_string(node, "qcom,ath10k-default-bdf",
1096 &board_name);
1097 if (!board_name)
1098 return -ENODATA;
1099
1100 if (strscpy(ar->id.default_bdf,
1101 board_name, sizeof(ar->id.default_bdf)) < 0)
1102 ath10k_warn(ar,
> 1103 "default board name is longer than allocated buffer, board_name: %s; allocated size: %d\n",
1104 board_name, sizeof(ar->id.default_bdf));
1105
1106 return 0;
1107 }
1108 EXPORT_SYMBOL(ath10k_core_parse_default_bdf_dt);
1109
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
Abhishek Kumar <[email protected]> writes:
> It is possible that BDF name with board-id+chip-id+variant
> combination is not found in the board-2.bin. Such cases can
> cause wlan probe to fail and completely break wifi. In such
> case there can be an optional property to define a default
> BDF name to search for in the board-2.bin file when none of
> the combinations (board-id,chip-id,variant) match.
> To address the above concern provide an optional proptery:
> qcom,ath10k-default-bdf
>
> Signed-off-by: Abhishek Kumar <[email protected]>
> ---
>
> .../devicetree/bindings/net/wireless/qcom,ath10k.txt | 4 ++++
> 1 file changed, 4 insertions(+)
Please CC ath10k list on all ath10k related patches.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Hi,
On Thu, Mar 10, 2022 at 2:06 AM Kalle Valo <[email protected]> wrote:
>
> Doug Anderson <[email protected]> writes:
>
> > Hi,
> >
> > On Fri, Jan 7, 2022 at 12:05 PM Abhishek Kumar <[email protected]> wrote:
> >>
> >> There can be cases where the board-2.bin does not contain
> >> any BDF matching the chip-id+board-id+variant combination.
> >> This causes the wlan probe to fail and renders wifi unusable.
> >> For e.g. if the board-2.bin has default BDF as:
> >> bus=snoc,qmi-board-id=67 but for some reason the board-id
> >> on the wlan chip is not programmed and read 0xff as the
> >> default value. In such cases there won't be any matching BDF
> >> because the board-2.bin will be searched with following:
> >> bus=snoc,qmi-board-id=ff
>
> I just checked, in ath10k-firmware WCN3990/hw1.0/board-2.bin we have
> that entry:
>
> BoardNames[1]: 'bus=snoc,qmi-board-id=ff'
>
> >> To address these scenarios, there can be an option to provide
> >> fallback default BDF name in the device tree. If none of the
> >> BDF names match then the board-2.bin file can be searched with
> >> default BDF names assigned in the device tree.
> >>
> >> The default BDF name can be set as:
> >> wifi@a000000 {
> >> status = "okay";
> >> qcom,ath10k-default-bdf = "bus=snoc,qmi-board-id=67";
> >
> > Rather than add a new device tree property, wouldn't it be good enough
> > to leverage the existing variant?
>
> I'm not thrilled either adding this to Device Tree, we should keep the
> bindings as simple as possible.
>
> > Right now I think that the board file contains:
> >
> > 'bus=snoc,qmi-board-id=67.bin'
> > 'bus=snoc,qmi-board-id=67,qmi-chip-id=320,variant=GO_LAZOR.bin'
> > 'bus=snoc,qmi-board-id=67,qmi-chip-id=320,variant=GO_POMPOM.bin'
> > 'bus=snoc,qmi-board-id=67,qmi-chip-id=4320,variant=GO_LAZOR.bin'
> > 'bus=snoc,qmi-board-id=67,qmi-chip-id=4320,variant=GO_POMPOM.bin'
> >
> > ...and, on lazor for instance, we have:
> >
> > qcom,ath10k-calibration-variant = "GO_LAZOR";
> >
> > The problem you're trying to solve is that on some early lazor
> > prototype hardware we didn't have the "board-id" programmed we could
> > get back 0xff from the hardware. As I understand it 0xff always means
> > "unprogrammed".
> >
> > It feels like you could just have a special case such that if the
> > hardware reports board ID of 0xff and you don't get a "match" that you
> > could just treat 0xff as a wildcard. That means that you'd see the
> > "variant" in the device tree and pick one of the "GO_LAZOR" entries.
> >
> > Anyway, I guess it's up to the people who spend more time in this file
> > which they'd prefer, but that seems like it'd be simple and wouldn't
> > require a bindings addition...
>
> In ath11k we need something similar for that I have been thinking like
> this:
>
> 'bus=snoc,qmi-board-id=67,qmi-chip-id=320,variant=GO_LAZOR'
>
> 'bus=snoc,qmi-board-id=67,qmi-chip-id=320'
>
> 'bus=snoc,qmi-board-id=67'
>
> 'bus=snoc'
>
> Ie. we drop one attribute at a time and try to find a suitable board
> file. Though I'm not sure if it's possible to find a board file which
> works with many different hardware, but the principle would be at least
> that. Would something like that work in your case?
I'll leave it for Abhishek to comment for sure since he's been the one
actively involved in keeping track of our board-2.bin file. As far as
I know:
* Mostly we need this for pre-production hardware that developers
inside Google and Qualcomm still have sitting around on their desks. I
guess some people are even still using this pre-production hardware to
surf the web?
* In the ideal world, we think that the best calibration would be to
use the board-specific one in these cases. AKA if board_id is 0xff
(unprogrammed) and variant is "GO_LAZOR" then the best solution would
be to use the settings for board id 0x67 and variant "GO_LAZOR". This
_ought_ to be better settings than the default 0xff settings without
the "GO_LAZOR".
* In reality, we're probably OK as long as _some_ reasonable settings
get picked. WiFi might not be super range optimized but at least it
will still come up and work.
-Doug
On Tue, Apr 12, 2022 at 3:47 AM Kalle Valo <[email protected]> wrote:
>
> Abhishek Kumar <[email protected]> writes:
>
> > Hi All,
> >
> > Apologies for the late reply, too many things at the same time.
>
> Trust me, I know the feeling :)
>
> > Just a quick note, Qcomm provided a new BDF file with support for
> > 'bus=snoc,qmi-board-id=ff' as well, so even without this patch, the
> > non-programmed devices(with board-id=0xff) would work. However, for
> > optimization of the BDF search strategy, the above mentioned strategy
> > would still not work: - The stripping of full Board name would not
> > work if board-id itself is not programmed i.e. =0xff e.g. for
> > 'bus=snoc,qmi-board-id=ff,qmi-chip-id=320,variant=GO_LAZOR' => no
> > match 'bus=snoc,qmi-board-id=ff,qmi-chip-id=320' => no match
> > 'bus=snoc,qmi-board-id=ff' => no match 'bus=snoc' => no match because
> > all the BDFs contains board-id=67
>
> Sorry, not fully following your here. Are you saying that the problem is
> that WCN3990/hw1.0/board-2.bin is missing board file for 'bus=snoc'?
Ya, that is what I meant here, the board-2.bin file does not contain
an entry for 'bus=snoc' and so if board-id=oxff then still there
cannot be any BDF that matches. So adding BDF for 'bus=snoc' would
simplify the approach.
> That's easy to add, each board file within board-2.bin has multiple
> names so we can easily select one board file for which we add
> 'bus=snoc'.
>
> > So with this DT patch specifically for case 'bus=snoc,qmi-board-id=ff'
> > => no match, we fallback to default case ( the one provided in DT)
> > i.e. 'bus=snoc,qmi-board-id=67' => match . I do not see how exactly
> > the driver can know that it should check for a board-id of 67.
>
> Sorry, not following you here either. Why would the driver need to use
> board-id 67?
>
> > However, to still remove dependency on the DT, we can make the
> > board-id as no-op if it is not programmed i.e. if the board-id=ff then
> > we would pick any BDF that match 'bus=snoc,qmi-board-id=<XX>' where XX
> > can be any arbitrary number. Thoughts ?
>
> To me using just 'bus=snoc' is more logical than picking up an arbitrary
> number. But I might be missing something here.
The reason I mentioned that if the board-id=oxff then pick any
available BDF entry which matches
'bus=snoc,qmi-board-id=<XX>' is because:
- This will atleast let the wlan chip to boot.
- There is no BDF for 'bus=snoc' , so further stripping of boardname
will not find any match, but as you mentioned that BDF for 'bus=snoc'
can be added, then this will make the logic simpler and we don't have
to pick 'bus=snoc,qmi-board-id=<XX>' with any arbitrary board-id.
I will rollout a patch with this approach.
>
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Thanks
Abhishek