* [PATCH 1/2] ath10k: search for default BDF name provided in DT @ 2022-01-07 20:04 Abhishek Kumar 2022-01-07 20:04 ` [PATCH 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-07 20:04 UTC (permalink / raw) To: kvalo, dianders Cc: pillair, kuabhs, linux-kernel, linux-wireless, David S. Miller, Jakub Kicinski, Kalle Valo, ath10k, 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> --- 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] dt: bindings: add dt entry for ath10k default BDF name 2022-01-07 20:04 [PATCH 1/2] ath10k: search for default BDF name provided in DT Abhishek Kumar @ 2022-01-07 20:04 ` Abhishek Kumar 2022-01-10 10:18 ` Kalle Valo 2022-01-07 21:22 ` [PATCH 1/2] ath10k: search for default BDF name provided in DT Doug Anderson 2022-01-08 3:14 ` kernel test robot 2 siblings, 1 reply; 10+ messages in thread From: Abhishek Kumar @ 2022-01-07 20:04 UTC (permalink / raw) To: kvalo, dianders Cc: pillair, kuabhs, linux-kernel, linux-wireless, 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> --- .../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 2/2] dt: bindings: add dt entry for ath10k default BDF name 2022-01-07 20:04 ` [PATCH 2/2] dt: bindings: add dt entry for ath10k default BDF name Abhishek Kumar @ 2022-01-10 10:18 ` Kalle Valo 0 siblings, 0 replies; 10+ messages in thread From: Kalle Valo @ 2022-01-10 10:18 UTC (permalink / raw) To: Abhishek Kumar Cc: dianders, pillair, linux-kernel, linux-wireless, David S. Miller, Jakub Kicinski, Rob Herring, devicetree, netdev, ath10k Abhishek Kumar <kuabhs@chromium.org> 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 <kuabhs@chromium.org> > --- > > .../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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] ath10k: search for default BDF name provided in DT 2022-01-07 20:04 [PATCH 1/2] ath10k: search for default BDF name provided in DT Abhishek Kumar 2022-01-07 20:04 ` [PATCH 2/2] dt: bindings: add dt entry for ath10k default BDF name Abhishek Kumar @ 2022-01-07 21:22 ` Doug Anderson 2022-03-10 10:05 ` Kalle Valo 2022-01-08 3:14 ` kernel test robot 2 siblings, 1 reply; 10+ messages in thread From: Doug Anderson @ 2022-01-07 21:22 UTC (permalink / raw) To: Abhishek Kumar Cc: Kalle Valo, Rakesh Pillai, LKML, linux-wireless, David S. Miller, Jakub Kicinski, Kalle Valo, ath10k, netdev Hi, On Fri, Jan 7, 2022 at 12:05 PM Abhishek Kumar <kuabhs@chromium.org> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] ath10k: search for default BDF name provided in DT 2022-01-07 21:22 ` [PATCH 1/2] ath10k: search for default BDF name provided in DT Doug Anderson @ 2022-03-10 10:05 ` Kalle Valo 2022-03-11 0:27 ` Doug Anderson 0 siblings, 1 reply; 10+ messages in thread From: Kalle Valo @ 2022-03-10 10:05 UTC (permalink / raw) To: Doug Anderson Cc: Abhishek Kumar, Rakesh Pillai, LKML, linux-wireless, David S. Miller, Jakub Kicinski, ath10k, netdev Doug Anderson <dianders@chromium.org> writes: > Hi, > > On Fri, Jan 7, 2022 at 12:05 PM Abhishek Kumar <kuabhs@chromium.org> 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? -- 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 1/2] ath10k: search for default BDF name provided in DT 2022-03-10 10:05 ` Kalle Valo @ 2022-03-11 0:27 ` Doug Anderson 2022-04-11 23:25 ` Abhishek Kumar 0 siblings, 1 reply; 10+ messages in thread From: Doug Anderson @ 2022-03-11 0:27 UTC (permalink / raw) To: Kalle Valo Cc: Abhishek Kumar, Rakesh Pillai, LKML, linux-wireless, David S. Miller, Jakub Kicinski, ath10k, netdev Hi, On Thu, Mar 10, 2022 at 2:06 AM Kalle Valo <kvalo@kernel.org> wrote: > > Doug Anderson <dianders@chromium.org> writes: > > > Hi, > > > > On Fri, Jan 7, 2022 at 12:05 PM Abhishek Kumar <kuabhs@chromium.org> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] ath10k: search for default BDF name provided in DT 2022-03-11 0:27 ` Doug Anderson @ 2022-04-11 23:25 ` Abhishek Kumar 2022-04-12 10:46 ` Kalle Valo 0 siblings, 1 reply; 10+ messages in thread From: Abhishek Kumar @ 2022-04-11 23:25 UTC (permalink / raw) To: Doug Anderson Cc: Kalle Valo, Rakesh Pillai, LKML, linux-wireless, David S. Miller, Jakub Kicinski, ath10k, netdev Hi All, Apologies for the late reply, too many things at the same time. 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 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. 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 ? -Abhishek On Thu, Mar 10, 2022 at 4:28 PM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Thu, Mar 10, 2022 at 2:06 AM Kalle Valo <kvalo@kernel.org> wrote: > > > > Doug Anderson <dianders@chromium.org> writes: > > > > > Hi, > > > > > > On Fri, Jan 7, 2022 at 12:05 PM Abhishek Kumar <kuabhs@chromium.org> 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] ath10k: search for default BDF name provided in DT 2022-04-11 23:25 ` Abhishek Kumar @ 2022-04-12 10:46 ` Kalle Valo 2022-04-20 0:43 ` Abhishek Kumar 0 siblings, 1 reply; 10+ messages in thread From: Kalle Valo @ 2022-04-12 10:46 UTC (permalink / raw) To: Abhishek Kumar Cc: Doug Anderson, Rakesh Pillai, LKML, linux-wireless, David S. Miller, Jakub Kicinski, ath10k, netdev Abhishek Kumar <kuabhs@chromium.org> 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'? 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. -- 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 1/2] ath10k: search for default BDF name provided in DT 2022-04-12 10:46 ` Kalle Valo @ 2022-04-20 0:43 ` Abhishek Kumar 0 siblings, 0 replies; 10+ messages in thread From: Abhishek Kumar @ 2022-04-20 0:43 UTC (permalink / raw) To: Kalle Valo Cc: Doug Anderson, Rakesh Pillai, LKML, linux-wireless, David S. Miller, Jakub Kicinski, ath10k, netdev On Tue, Apr 12, 2022 at 3:47 AM Kalle Valo <kvalo@kernel.org> wrote: > > Abhishek Kumar <kuabhs@chromium.org> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] ath10k: search for default BDF name provided in DT 2022-01-07 20:04 [PATCH 1/2] ath10k: search for default BDF name provided in DT Abhishek Kumar 2022-01-07 20:04 ` [PATCH 2/2] dt: bindings: add dt entry for ath10k default BDF name Abhishek Kumar 2022-01-07 21:22 ` [PATCH 1/2] ath10k: search for default BDF name provided in DT Doug Anderson @ 2022-01-08 3:14 ` kernel test robot 2 siblings, 0 replies; 10+ messages in thread From: kernel test robot @ 2022-01-08 3:14 UTC (permalink / raw) To: Abhishek Kumar, kvalo, dianders Cc: kbuild-all, pillair, kuabhs, linux-kernel, linux-wireless, Jakub Kicinski, Kalle Valo, ath10k, 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-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/202201081118.iN7Rcf4J-lkp@intel.com/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 <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: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/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-04-20 0:43 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-07 20:04 [PATCH 1/2] ath10k: search for default BDF name provided in DT Abhishek Kumar 2022-01-07 20:04 ` [PATCH 2/2] dt: bindings: add dt entry for ath10k default BDF name Abhishek Kumar 2022-01-10 10:18 ` Kalle Valo 2022-01-07 21:22 ` [PATCH 1/2] ath10k: search for default BDF name provided in DT Doug Anderson 2022-03-10 10:05 ` Kalle Valo 2022-03-11 0:27 ` Doug Anderson 2022-04-11 23:25 ` Abhishek Kumar 2022-04-12 10:46 ` Kalle Valo 2022-04-20 0:43 ` Abhishek Kumar 2022-01-08 3:14 ` kernel test robot
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).