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]>
---
Changes in v2: Fix printf formatting issue.
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..756856a8eed3 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: %ld\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]>
---
Changes in v2:
- Changes in v2: none
.../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 Mon, Jan 10, 2022 at 3:15 PM Abhishek Kumar <[email protected]> wrote:
>
> +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: %ld\n",
> + board_name, sizeof(ar->id.default_bdf));
I suspect, but don't know for sure, that you're going to get another
builder splat here. Just like sizeof() isn't guaranteed to return an
"unsigned int", it's also not guaranteed to return an "unsigned long".
I believe you want %zu. See Documentation/core-api/printk-formats.rst
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(ath10k_core_parse_default_bdf_dt);
Boy, that function seems like overkill for something that you need
once at init time. ...and I also suspect that the lifetime of the
string returned by of_property_read_string() is valid for as long as
your "of_node" is held and thus probably you could use it directly (it
likely has a longer lifetime than the location you're storing it).
...but I guess it matches the ath10k_core_check_dt() function above
it, so I guess it's fine?
-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 next-20220110]
[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/20220111-071636
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git ath-next
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220111/[email protected]/config)
compiler: arceb-elf-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/50c4c7cb02cc786afcd9aff27616a6e20296c703
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/20220111-071636
git checkout 50c4c7cb02cc786afcd9aff27616a6e20296c703
# 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=arc 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:116: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'unsigned int' [-Wformat=]
1103 | "default board name is longer than allocated buffer, board_name: %s; allocated size: %ld\n",
| ~~^
| |
| long int
| %d
1104 | board_name, sizeof(ar->id.default_bdf));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| 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: %ld\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]
On Mon, 10 Jan 2022 23:14:15 +0000, Abhishek Kumar wrote:
> 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]>
> ---
>
> Changes in v2:
> - Changes in v2: none
>
> .../devicetree/bindings/net/wireless/qcom,ath10k.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
Hi Reviewers,
On this patch I have a kernel bot warning, which I intend to fix along
with all the comments and discussion and push out V3. So, any
comments/next steps are appreciated.
Thanks
Abhishek
On Mon, Jan 10, 2022 at 5:08 PM kernel test robot <[email protected]> wrote:
>
> 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 next-20220110]
> [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/20220111-071636
> base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git ath-next
> config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220111/[email protected]/config)
> compiler: arceb-elf-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/50c4c7cb02cc786afcd9aff27616a6e20296c703
> 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/20220111-071636
> git checkout 50c4c7cb02cc786afcd9aff27616a6e20296c703
> # 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=arc 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:116: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'unsigned int' [-Wformat=]
> 1103 | "default board name is longer than allocated buffer, board_name: %s; allocated size: %ld\n",
> | ~~^
> | |
> | long int
> | %d
> 1104 | board_name, sizeof(ar->id.default_bdf));
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~
> | |
> | 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: %ld\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]
On Mon, Jan 10, 2022 at 4:51 PM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Mon, Jan 10, 2022 at 3:15 PM Abhishek Kumar <[email protected]> wrote:
> >
> > +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: %ld\n",
> > + board_name, sizeof(ar->id.default_bdf));
>
> I suspect, but don't know for sure, that you're going to get another
> builder splat here. Just like sizeof() isn't guaranteed to return an
> "unsigned int", it's also not guaranteed to return an "unsigned long".
> I believe you want %zu. See Documentation/core-api/printk-formats.rst
Thanks for the tip, I will make this fix in V3.
>
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(ath10k_core_parse_default_bdf_dt);
>
> Boy, that function seems like overkill for something that you need
> once at init time. ...and I also suspect that the lifetime of the
> string returned by of_property_read_string() is valid for as long as
> your "of_node" is held and thus probably you could use it directly (it
> likely has a longer lifetime than the location you're storing it).
>
> ...but I guess it matches the ath10k_core_check_dt() function above
> it, so I guess it's fine?
Ya, that was my idea to match it with ath10k_core_check_dt, initially,
I was planning to remodify ath10k_core_check_dt to parse the new
property, but looks it is used it multiple places, so I thought having
a separate parser function would be cleaner, however, I am open to new
ideas.
- Abhishek
Abhishek Kumar <[email protected]> writes:
> On this patch I have a kernel bot warning, which I intend to fix along
> with all the comments and discussion and push out V3. So, any
> comments/next steps are appreciated.
Please wait my comments before sending v3, I think this is something
which is also needed in ath11k and I need to look at it in detail.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Kalle,
On Fri, Jan 14, 2022 at 6:46 AM Kalle Valo <[email protected]> wrote:
>
> Abhishek Kumar <[email protected]> writes:
>
> > On this patch I have a kernel bot warning, which I intend to fix along
> > with all the comments and discussion and push out V3. So, any
> > comments/next steps are appreciated.
>
> Please wait my comments before sending v3, I think this is something
> which is also needed in ath11k and I need to look at it in detail.
I'm wondering if you have a timeframe for when you might post your
comments. We've landed this patch locally in the Chrome OS kernel
tree, but we are always also interested in it landing upstream. If you
have ideas for a path forward that'd be keen!
-Doug