linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] ath10k: search for default BDF name provided in DT
@ 2022-01-10 23:14 Abhishek Kumar
  2022-01-10 23:14 ` [PATCH v2 2/2] dt: bindings: add dt entry for ath10k default BDF name Abhishek Kumar
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Abhishek Kumar @ 2022-01-10 23:14 UTC (permalink / raw)
  To: kvalo, ath10k
  Cc: pillair, linux-wireless, linux-kernel, kuabhs, dianders,
	David S. Miller, Jakub Kicinski, Kalle Valo, netdev

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 <kuabhs@chromium.org>
---

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] dt: bindings: add dt entry for ath10k default BDF name
  2022-01-10 23:14 [PATCH v2 1/2] ath10k: search for default BDF name provided in DT Abhishek Kumar
@ 2022-01-10 23:14 ` Abhishek Kumar
  2022-01-12  2:21   ` Rob Herring
  2022-01-11  0:51 ` [PATCH v2 1/2] ath10k: search for default BDF name provided in DT Doug Anderson
  2022-01-11  1:07 ` kernel test robot
  2 siblings, 1 reply; 10+ messages in thread
From: Abhishek Kumar @ 2022-01-10 23:14 UTC (permalink / raw)
  To: kvalo, ath10k
  Cc: pillair, linux-wireless, linux-kernel, kuabhs, dianders,
	David S. Miller, Jakub Kicinski, Kalle Valo, Rob Herring,
	devicetree, netdev

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 <kuabhs@chromium.org>
---

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] ath10k: search for default BDF name provided in DT
  2022-01-10 23:14 [PATCH v2 1/2] ath10k: search for default BDF name provided in DT Abhishek Kumar
  2022-01-10 23:14 ` [PATCH v2 2/2] dt: bindings: add dt entry for ath10k default BDF name Abhishek Kumar
@ 2022-01-11  0:51 ` Doug Anderson
  2022-01-14  6:50   ` Abhishek Kumar
  2022-01-11  1:07 ` kernel test robot
  2 siblings, 1 reply; 10+ messages in thread
From: Doug Anderson @ 2022-01-11  0:51 UTC (permalink / raw)
  To: Abhishek Kumar
  Cc: Kalle Valo, ath10k, Rakesh Pillai, linux-wireless, LKML,
	David S. Miller, Jakub Kicinski, Kalle Valo, netdev

Hi,

On Mon, Jan 10, 2022 at 3:15 PM Abhishek Kumar <kuabhs@chromium.org> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] ath10k: search for default BDF name provided in DT
  2022-01-10 23:14 [PATCH v2 1/2] ath10k: search for default BDF name provided in DT Abhishek Kumar
  2022-01-10 23:14 ` [PATCH v2 2/2] dt: bindings: add dt entry for ath10k default BDF name Abhishek Kumar
  2022-01-11  0:51 ` [PATCH v2 1/2] ath10k: search for default BDF name provided in DT Doug Anderson
@ 2022-01-11  1:07 ` kernel test robot
  2022-01-14  6:47   ` Abhishek Kumar
  2 siblings, 1 reply; 10+ messages in thread
From: kernel test robot @ 2022-01-11  1:07 UTC (permalink / raw)
  To: Abhishek Kumar, kvalo, ath10k
  Cc: kbuild-all, pillair, linux-wireless, linux-kernel, kuabhs,
	dianders, Jakub Kicinski, Kalle Valo, netdev

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/202201110851.5qAxfQJj-lkp@intel.com/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 <lkp@intel.com>

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/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] dt: bindings: add dt entry for ath10k default BDF name
  2022-01-10 23:14 ` [PATCH v2 2/2] dt: bindings: add dt entry for ath10k default BDF name Abhishek Kumar
@ 2022-01-12  2:21   ` Rob Herring
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2022-01-12  2:21 UTC (permalink / raw)
  To: Abhishek Kumar
  Cc: devicetree, netdev, pillair, Kalle Valo, linux-wireless, ath10k,
	David S. Miller, Rob Herring, Jakub Kicinski, dianders, kvalo,
	linux-kernel

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 <kuabhs@chromium.org>
> ---
> 
> Changes in v2:
>  - Changes in v2: none
> 
>  .../devicetree/bindings/net/wireless/qcom,ath10k.txt          | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] ath10k: search for default BDF name provided in DT
  2022-01-11  1:07 ` kernel test robot
@ 2022-01-14  6:47   ` Abhishek Kumar
  2022-01-14 14:46     ` Kalle Valo
  0 siblings, 1 reply; 10+ messages in thread
From: Abhishek Kumar @ 2022-01-14  6:47 UTC (permalink / raw)
  To: kernel test robot
  Cc: kvalo, ath10k, kbuild-all, pillair, linux-wireless, linux-kernel,
	dianders, Jakub Kicinski, Kalle Valo, netdev

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 <lkp@intel.com> 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/202201110851.5qAxfQJj-lkp@intel.com/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 <lkp@intel.com>
>
> 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/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] ath10k: search for default BDF name provided in DT
  2022-01-11  0:51 ` [PATCH v2 1/2] ath10k: search for default BDF name provided in DT Doug Anderson
@ 2022-01-14  6:50   ` Abhishek Kumar
  0 siblings, 0 replies; 10+ messages in thread
From: Abhishek Kumar @ 2022-01-14  6:50 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Kalle Valo, ath10k, Rakesh Pillai, linux-wireless, LKML,
	David S. Miller, Jakub Kicinski, Kalle Valo, netdev

On Mon, Jan 10, 2022 at 4:51 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Jan 10, 2022 at 3:15 PM Abhishek Kumar <kuabhs@chromium.org> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] ath10k: search for default BDF name provided in DT
  2022-01-14  6:47   ` Abhishek Kumar
@ 2022-01-14 14:46     ` Kalle Valo
  2022-03-08  0:50       ` Doug Anderson
  0 siblings, 1 reply; 10+ messages in thread
From: Kalle Valo @ 2022-01-14 14:46 UTC (permalink / raw)
  To: Abhishek Kumar
  Cc: kernel test robot, ath10k, kbuild-all, pillair, linux-wireless,
	linux-kernel, dianders, Jakub Kicinski, netdev

Abhishek Kumar <kuabhs@chromium.org> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] ath10k: search for default BDF name provided in DT
  2022-01-14 14:46     ` Kalle Valo
@ 2022-03-08  0:50       ` Doug Anderson
  2022-03-10 10:07         ` Kalle Valo
  0 siblings, 1 reply; 10+ messages in thread
From: Doug Anderson @ 2022-03-08  0:50 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Abhishek Kumar, kernel test robot, ath10k, kbuild-all,
	Rakesh Pillai, linux-wireless, LKML, Jakub Kicinski, netdev

Kalle,

On Fri, Jan 14, 2022 at 6:46 AM Kalle Valo <kvalo@kernel.org> wrote:
>
> Abhishek Kumar <kuabhs@chromium.org> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] ath10k: search for default BDF name provided in DT
  2022-03-08  0:50       ` Doug Anderson
@ 2022-03-10 10:07         ` Kalle Valo
  0 siblings, 0 replies; 10+ messages in thread
From: Kalle Valo @ 2022-03-10 10:07 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Abhishek Kumar, kernel test robot, ath10k, kbuild-all,
	Rakesh Pillai, linux-wireless, LKML, Jakub Kicinski, netdev

Doug Anderson <dianders@chromium.org> writes:

> On Fri, Jan 14, 2022 at 6:46 AM Kalle Valo <kvalo@kernel.org> wrote:
>>
>> Abhishek Kumar <kuabhs@chromium.org> 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!

You had a good comment on v1 so I replied to that one.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-03-10 10:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-10 23:14 [PATCH v2 1/2] ath10k: search for default BDF name provided in DT Abhishek Kumar
2022-01-10 23:14 ` [PATCH v2 2/2] dt: bindings: add dt entry for ath10k default BDF name Abhishek Kumar
2022-01-12  2:21   ` Rob Herring
2022-01-11  0:51 ` [PATCH v2 1/2] ath10k: search for default BDF name provided in DT Doug Anderson
2022-01-14  6:50   ` Abhishek Kumar
2022-01-11  1:07 ` kernel test robot
2022-01-14  6:47   ` Abhishek Kumar
2022-01-14 14:46     ` Kalle Valo
2022-03-08  0:50       ` Doug Anderson
2022-03-10 10:07         ` Kalle Valo

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).