Re: [PATCH RFC 4/5] wifi: ath12k: Support MAC address from the firmware node

[Paul Menzel <pmenzel@molgen.mpg.de>]

Dear Abel,


Thank you for your patch.

Am 14.10.26 um 15:02 schrieb Abel Vesa:
> Read a MAC address from the device's firmware node (a DT "mac-address"/
> "local-mac-address" property, or an nvmem cell named "mac-address" via
> device_get_mac_address()), and use it in preference to the address from
> the firmware board data. This mirrors what ath10k and ath11k already do
> and lets the WLAN MAC be sourced from firmware-provisioned storage such
> as a UEFI variable exposed through nvmem.
> 
> A device tree / nvmem address takes precedence over the board-data MAC,
> so an explicitly provisioned address wins even when the firmware reports
> a valid per-pdev address. An all-zero (absent) address is ignored.
> 
> Note: the provisioned address must be a valid unicast MAC; a multicast
> address (odd first octet) is rejected by nvmem_get_mac_address().

Please add a paragraph how you tested this.

> Assisted-by: LLM
> Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> ---
>   drivers/net/wireless/ath/ath12k/mac.c | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 01f8c6572f65..f4d2d0377c8e 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -14878,15 +14878,27 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
>   	u32 ht_cap = U32_MAX, antennas_rx = 0, antennas_tx = 0;
>   	bool is_6ghz = false, is_raw_mode = false, is_monitor_disable = false;
>   	u8 *mac_addr = NULL;
> +	u8 mac_from_dt[ETH_ALEN] = {};
>   	u8 mbssid_max_interfaces = 0;
>   
>   	wiphy->max_ap_assoc_sta = 0;
>   
> +	/*
> +	 * Allow a MAC address provided through the device tree (directly or
> +	 * via an nvmem cell named "mac-address") to override the one from the
> +	 * firmware board data.
> +	 */
> +	device_get_mac_address(ab->dev, mac_from_dt);
> +
>   	for_each_ar(ah, ar, i) {
>   		u32 ht_cap_info = 0;
>   
>   		pdev = ar->pdev;
> -		if (ar->ab->pdevs_macaddr_valid) {
> +		if (!is_zero_ether_addr(mac_from_dt)) {
> +			/* A device tree / nvmem address takes precedence. */

The comment is redundant as the code ordering shows exactly this.

> +			ether_addr_copy(ar->mac_addr, mac_from_dt);
> +			ar->mac_addr[4] += ar->pdev_idx;
> +		} else if (ar->ab->pdevs_macaddr_valid) {
>   			ether_addr_copy(ar->mac_addr, pdev->mac_addr);
>   		} else {
>   			ether_addr_copy(ar->mac_addr, ar->ab->mac_addr);
> 

With the commit message extended, and the comment removed:

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul