netdev - Re: [PATCH RFC 2/4] wifi: ath10k: support board-specific firmware overrides

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Fri, 01 Mar 2024 10:01:17 +0200
From: Kalle Valo <kvalo@...nel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Jeff Johnson <quic_jjohnson@...cinc.com>,  "David S. Miller"
 <davem@...emloft.net>,  Eric Dumazet <edumazet@...gle.com>,  Jakub
 Kicinski <kuba@...nel.org>,  Paolo Abeni <pabeni@...hat.com>,  Rob Herring
 <robh+dt@...nel.org>,  Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@...aro.org>,  Conor Dooley <conor+dt@...nel.org>,
  Bjorn Andersson <andersson@...nel.org>,  Konrad Dybcio
 <konrad.dybcio@...aro.org>,  ath10k@...ts.infradead.org,
  linux-wireless@...r.kernel.org,  netdev@...r.kernel.org,
  devicetree@...r.kernel.org,  linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH RFC 2/4] wifi: ath10k: support board-specific firmware
 overrides

Dmitry Baryshkov <dmitry.baryshkov@...aro.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@...aro.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()?

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

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