wifi: ath10k: switch to of_get_mac_address

[PATCH ath-next] wifi: ath10k: switch to of_get_mac_address

Posted by Rosen Penev 1 month, 3 weeks ago

In 9d5804662ce1f9bdde0a14c3c40940acbbf09538 , device_get_mac_address was
introduced as a generic way to get MAC addresses from anywhere.
Unfortunately since then, the landscape has changed and the OF version
is required for NVMEM support. The second problem is that with NVMEM
it's possible that it loads after ath10k. For that reason, check for
deferred errors and exit out of probe in such a case.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 6f78f1752cd6..76747eb0925b 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/firmware.h>
 #include <linux/of.h>
+#include <linux/of_net.h>
 #include <linux/property.h>
 #include <linux/dmi.h>
 #include <linux/ctype.h>
@@ -3456,7 +3457,9 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
 		ath10k_debug_print_board_info(ar);
 	}
 
-	device_get_mac_address(ar->dev, ar->mac_addr);
+	ret = of_get_mac_address(ar->dev->of_node, ar->mac_addr);
+	if (ret == -EPROBE_DEFER)
+		goto err_free_firmware_files;
 
 	ret = ath10k_core_init_firmware_features(ar);
 	if (ret) {
-- 
2.50.1

Re: [PATCH ath-next] wifi: ath10k: switch to of_get_mac_address

Posted by Jeff Johnson 1 month, 3 weeks ago

On 8/8/2025 8:15 PM, Rosen Penev wrote:
> In 9d5804662ce1f9bdde0a14c3c40940acbbf09538 , device_get_mac_address was

see
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes

specifically: If you want to refer to a specific commit, don’t just refer to
the SHA-1 ID of the commit. Please also include the oneline summary of the
commit, to make it easier for reviewers to know what it is about.

So in this case:
9d5804662ce1 ("ath10k: retrieve MAC address from system firmware if provided")

> introduced as a generic way to get MAC addresses from anywhere.
> Unfortunately since then, the landscape has changed and the OF version

when did the landscape change? if using device_get_mac_address() is breaking
folks, it would be nice to know which versions of the kernel have the bad
behavior so that the patch can be backported to any broken LTS kernels.

> is required for NVMEM support. The second problem is that with NVMEM
> it's possible that it loads after ath10k. For that reason, check for
> deferred errors and exit out of probe in such a case.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/net/wireless/ath/ath10k/core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> index 6f78f1752cd6..76747eb0925b 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -11,6 +11,7 @@
>  #include <linux/module.h>
>  #include <linux/firmware.h>
>  #include <linux/of.h>
> +#include <linux/of_net.h>
>  #include <linux/property.h>
>  #include <linux/dmi.h>
>  #include <linux/ctype.h>
> @@ -3456,7 +3457,9 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
>  		ath10k_debug_print_board_info(ar);
>  	}
>  
> -	device_get_mac_address(ar->dev, ar->mac_addr);
> +	ret = of_get_mac_address(ar->dev->of_node, ar->mac_addr);
> +	if (ret == -EPROBE_DEFER)
> +		goto err_free_firmware_files;

Note a similar proposal for ath11k was deferred since it seems to break x86
attachment when there isn't a device tree node:
https://msgid.link/ec974dc0-962b-f611-7bbb-c07a3872f70f@oss.qualcomm.com

I'd have the same concerns here.
(but I didn't dig into how the fwnode items are set if there isn't a DT)

>  
>  	ret = ath10k_core_init_firmware_features(ar);
>  	if (ret) {

Re: [PATCH ath-next] wifi: ath10k: switch to of_get_mac_address

Posted by Rosen Penev 1 month, 3 weeks ago

On Mon, Aug 11, 2025 at 12:55 PM Jeff Johnson
<jeff.johnson@oss.qualcomm.com> wrote:
>
> On 8/8/2025 8:15 PM, Rosen Penev wrote:
> > In 9d5804662ce1f9bdde0a14c3c40940acbbf09538 , device_get_mac_address was
>
> see
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>
> specifically: If you want to refer to a specific commit, don’t just refer to
> the SHA-1 ID of the commit. Please also include the oneline summary of the
> commit, to make it easier for reviewers to know what it is about.
>
> So in this case:
> 9d5804662ce1 ("ath10k: retrieve MAC address from system firmware if provided")
>
> > introduced as a generic way to get MAC addresses from anywhere.
> > Unfortunately since then, the landscape has changed and the OF version
>
> when did the landscape change? if using device_get_mac_address() is breaking
> folks, it would be nice to know which versions of the kernel have the bad
> behavior so that the patch can be backported to any broken LTS kernels.
>
> > is required for NVMEM support. The second problem is that with NVMEM
> > it's possible that it loads after ath10k. For that reason, check for
> > deferred errors and exit out of probe in such a case.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  drivers/net/wireless/ath/ath10k/core.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> > index 6f78f1752cd6..76747eb0925b 100644
> > --- a/drivers/net/wireless/ath/ath10k/core.c
> > +++ b/drivers/net/wireless/ath/ath10k/core.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/module.h>
> >  #include <linux/firmware.h>
> >  #include <linux/of.h>
> > +#include <linux/of_net.h>
> >  #include <linux/property.h>
> >  #include <linux/dmi.h>
> >  #include <linux/ctype.h>
> > @@ -3456,7 +3457,9 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
> >               ath10k_debug_print_board_info(ar);
> >       }
> >
> > -     device_get_mac_address(ar->dev, ar->mac_addr);
> > +     ret = of_get_mac_address(ar->dev->of_node, ar->mac_addr);
> > +     if (ret == -EPROBE_DEFER)
> > +             goto err_free_firmware_files;
>
> Note a similar proposal for ath11k was deferred since it seems to break x86
> attachment when there isn't a device tree node:
> https://msgid.link/ec974dc0-962b-f611-7bbb-c07a3872f70f@oss.qualcomm.com
In response to this, I'll change this patch to only add NVMEM support
>
> I'd have the same concerns here.
> (but I didn't dig into how the fwnode items are set if there isn't a DT)
I have no idea either. I assume for pcie cards the MAC is set from the EEPROM.
>
> >
> >       ret = ath10k_core_init_firmware_features(ar);
> >       if (ret) {
>