| Message ID | 20240130-wcn3990-firmware-path-v1-2-826b93202964@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: ath10k: support board-specific firmware overrides | expand |
On 30.01.2024 17:38, Dmitry Baryshkov wrote: > Different Qualcomm platforms using WCN3990 WiFI chip use SoC-specific > firmware versions with different features. For example firmware for > SDM845 doesn't use single-chan-info-per-channel feature, while firmware > for QRB2210 / QRB4210 requires that feature. Allow board DT files to > override the subdir of the fw dir used to lookup the firmware-N.bin file > decribing corresponding WiFi firmware. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/net/wireless/ath/ath10k/core.c | 11 ++++++++++- > drivers/net/wireless/ath/ath10k/core.h | 2 ++ > drivers/net/wireless/ath/ath10k/snoc.c | 3 +++ > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c > index 0032f8aa892f..ef7ce8b3f8fb 100644 > --- a/drivers/net/wireless/ath/ath10k/core.c > +++ b/drivers/net/wireless/ath/ath10k/core.c > @@ -942,11 +942,20 @@ static const struct firmware *ath10k_fetch_fw_file(struct ath10k *ar, > if (dir == NULL) > dir = "."; > > + if (ar->board_name) { > + snprintf(filename, sizeof(filename), "%s/%s/%s", > + dir, ar->board_name, file); > + ret = firmware_request_nowarn(&fw, filename, ar->dev); > + ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot fw request '%s': %d\n", > + filename, ret); Perhaps it'd be useful to move to a more noisy loglevel Konrad
On Mon, 12 Feb 2024 at 13:12, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 30.01.2024 17:38, Dmitry Baryshkov wrote: > > Different Qualcomm platforms using WCN3990 WiFI chip use SoC-specific > > firmware versions with different features. For example firmware for > > SDM845 doesn't use single-chan-info-per-channel feature, while firmware > > for QRB2210 / QRB4210 requires that feature. Allow board DT files to > > override the subdir of the fw dir used to lookup the firmware-N.bin file > > decribing corresponding WiFi firmware. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/net/wireless/ath/ath10k/core.c | 11 ++++++++++- > > drivers/net/wireless/ath/ath10k/core.h | 2 ++ > > drivers/net/wireless/ath/ath10k/snoc.c | 3 +++ > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c > > index 0032f8aa892f..ef7ce8b3f8fb 100644 > > --- a/drivers/net/wireless/ath/ath10k/core.c > > +++ b/drivers/net/wireless/ath/ath10k/core.c > > @@ -942,11 +942,20 @@ static const struct firmware *ath10k_fetch_fw_file(struct ath10k *ar, > > if (dir == NULL) > > dir = "."; > > > > + if (ar->board_name) { > > + snprintf(filename, sizeof(filename), "%s/%s/%s", > > + dir, ar->board_name, file); > > + ret = firmware_request_nowarn(&fw, filename, ar->dev); > > + ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot fw request '%s': %d\n", > > + filename, ret); > > Perhaps it'd be useful to move to a more noisy loglevel No, these are details. If the firmware is in place, it is loaded properly.
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes: > Different Qualcomm platforms using WCN3990 WiFI chip use SoC-specific > firmware versions with different features. For example firmware for > SDM845 doesn't use single-chan-info-per-channel feature, while firmware > for QRB2210 / QRB4210 requires that feature. Allow board DT files to > override the subdir of the fw dir used to lookup the firmware-N.bin file > decribing corresponding WiFi firmware. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Sorry for the delay, too many drivers... But this looks good to me, few small comments. In the commit message it would it would be good to have an example of the new firmware path. And also mention that board file (board-2.bin) handling is not affected, at least that's how understood from reading the code. > --- a/drivers/net/wireless/ath/ath10k/core.c > +++ b/drivers/net/wireless/ath/ath10k/core.c > @@ -942,11 +942,20 @@ static const struct firmware *ath10k_fetch_fw_file(struct ath10k *ar, > if (dir == NULL) > dir = "."; > > + if (ar->board_name) { > + snprintf(filename, sizeof(filename), "%s/%s/%s", > + dir, ar->board_name, file); > + ret = firmware_request_nowarn(&fw, filename, ar->dev); > + ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot fw request '%s': %d\n", > + filename, ret); > + if (!ret) > + return fw; > + } So here you test if ar->board_name is NULL. > --- a/drivers/net/wireless/ath/ath10k/snoc.c > +++ b/drivers/net/wireless/ath/ath10k/snoc.c > @@ -1337,6 +1337,9 @@ static void ath10k_snoc_quirks_init(struct ath10k *ar) > struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar); > struct device *dev = &ar_snoc->dev->dev; > > + /* ignore errors, default to empty string */ > + of_property_read_string(dev->of_node, "firmware-name", &ar->board_name); What do you mean with empty string in this case, "\n" (with length of 1) or NULL? Should we also test for strlen(0) in ath10k_fetch_fw_file()?
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index 0032f8aa892f..ef7ce8b3f8fb 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -942,11 +942,20 @@ static const struct firmware *ath10k_fetch_fw_file(struct ath10k *ar, if (dir == NULL) dir = "."; + if (ar->board_name) { + snprintf(filename, sizeof(filename), "%s/%s/%s", + dir, ar->board_name, file); + ret = firmware_request_nowarn(&fw, filename, ar->dev); + ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot fw request '%s': %d\n", + filename, ret); + if (!ret) + return fw; + } + snprintf(filename, sizeof(filename), "%s/%s", dir, file); ret = firmware_request_nowarn(&fw, filename, ar->dev); ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot fw request '%s': %d\n", filename, ret); - if (ret) return ERR_PTR(ret); diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h index c110d15528bd..3595c8abce02 100644 --- a/drivers/net/wireless/ath/ath10k/core.h +++ b/drivers/net/wireless/ath/ath10k/core.h @@ -1081,6 +1081,8 @@ struct ath10k { */ const struct ath10k_fw_components *running_fw; + const char *board_name; + const struct firmware *pre_cal_file; const struct firmware *cal_file; diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c index a1db5a973780..747de30e06ca 100644 --- a/drivers/net/wireless/ath/ath10k/snoc.c +++ b/drivers/net/wireless/ath/ath10k/snoc.c @@ -1337,6 +1337,9 @@ static void ath10k_snoc_quirks_init(struct ath10k *ar) struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar); struct device *dev = &ar_snoc->dev->dev; + /* ignore errors, default to empty string */ + of_property_read_string(dev->of_node, "firmware-name", &ar->board_name); + if (of_property_read_bool(dev->of_node, "qcom,snoc-host-cap-8bit-quirk")) set_bit(ATH10K_SNOC_FLAG_8BIT_HOST_CAP_QUIRK, &ar_snoc->flags); }
Different Qualcomm platforms using WCN3990 WiFI chip use SoC-specific firmware versions with different features. For example firmware for SDM845 doesn't use single-chan-info-per-channel feature, while firmware for QRB2210 / QRB4210 requires that feature. Allow board DT files to override the subdir of the fw dir used to lookup the firmware-N.bin file decribing corresponding WiFi firmware. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/net/wireless/ath/ath10k/core.c | 11 ++++++++++- drivers/net/wireless/ath/ath10k/core.h | 2 ++ drivers/net/wireless/ath/ath10k/snoc.c | 3 +++ 3 files changed, 15 insertions(+), 1 deletion(-)