2018-08-10 23:41:35

by Brian Norris

[permalink] [raw]
Subject: [PATCH] ath10k: retrieve MAC address from firmware if provided

Devices may provide their own MAC address via system firmware (e.g.,
device tree), especially in the case where the device doesn't have a
useful EEPROM on which to store its MAC address (e.g., for integrated
Wifi).

Use the generic device helper to retrieve the MAC address, and (if
present) honor it above the MAC address advertised by the card.

Signed-off-by: Brian Norris <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 3 +++
drivers/net/wireless/ath/ath10k/wmi.c | 3 ++-
2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index c40cd129afe7..840c1f039098 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -19,6 +19,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
#include <linux/of.h>
+#include <linux/property.h>
#include <linux/dmi.h>
#include <linux/ctype.h>
#include <asm/byteorder.h>
@@ -2602,6 +2603,8 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
ath10k_debug_print_board_info(ar);
}

+ device_get_mac_address(ar->dev, ar->mac_addr, sizeof(ar->mac_addr));
+
ret = ath10k_core_init_firmware_features(ar);
if (ret) {
ath10k_err(ar, "fatal problem with firmware features: %d\n",
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index fd612d2905b0..3cfd98de8ad2 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -5449,7 +5449,8 @@ int ath10k_wmi_event_ready(struct ath10k *ar, struct sk_buff *skb)
arg.mac_addr,
__le32_to_cpu(arg.status));

- ether_addr_copy(ar->mac_addr, arg.mac_addr);
+ if (is_zero_ether_addr(ar->mac_addr))
+ ether_addr_copy(ar->mac_addr, arg.mac_addr);
complete(&ar->wmi.unified_ready);
return 0;
}
--
2.18.0.597.ga71716f1ad-goog



2018-08-11 20:16:33

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH] ath10k: retrieve MAC address from firmware if provided

On 8/11/2018 1:39 AM, Brian Norris wrote:
> Devices may provide their own MAC address via system firmware (e.g.,

You got me confused by using just "firmware" in the subject.

> device tree), especially in the case where the device doesn't have a
> useful EEPROM on which to store its MAC address (e.g., for integrated
> Wifi).
>
> Use the generic device helper to retrieve the MAC address, and (if
> present) honor it above the MAC address advertised by the card.

But this put me back on track ;-)

> Signed-off-by: Brian Norris <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/core.c | 3 +++
> drivers/net/wireless/ath/ath10k/wmi.c | 3 ++-
> 2 files changed, 5 insertions(+), 1 deletion(-)


2018-08-13 17:56:52

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH] ath10k: retrieve MAC address from firmware if provided

On Sat, Aug 11, 2018 at 11:26 AM Arend van Spriel
<[email protected]> wrote:
>
> On 8/11/2018 1:39 AM, Brian Norris wrote:
> > Devices may provide their own MAC address via system firmware (e.g.,
>
> You got me confused by using just "firmware" in the subject.

Yeah...I started by writing this patch with device tree specifically
(of_get_mac_address()), and then later found that there were generic
"device" helpers for this, which can assist with other sorts of
firmware nodes. It was easier to put a name on a device tree patch
than on a "device" patch. I suppose "system firmware" might be a
better description?

> > device tree), especially in the case where the device doesn't have a
> > useful EEPROM on which to store its MAC address (e.g., for integrated
> > Wifi).
> >
> > Use the generic device helper to retrieve the MAC address, and (if
> > present) honor it above the MAC address advertised by the card.
>
> But this put me back on track ;-)

Let me know if you have a better way to clarify. I can resend with a
slightly modified subject (s/firmware/system firmware/), or let Kalle
do it, if that's the only thing to change.

Brian

2018-08-13 18:59:40

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH] ath10k: retrieve MAC address from firmware if provided

On 8/13/2018 7:14 PM, Brian Norris wrote:
> On Sat, Aug 11, 2018 at 11:26 AM Arend van Spriel
> <[email protected]> wrote:
>>
>> On 8/11/2018 1:39 AM, Brian Norris wrote:
>>> Devices may provide their own MAC address via system firmware (e.g.,
>>
>> You got me confused by using just "firmware" in the subject.
>
> Yeah...I started by writing this patch with device tree specifically
> (of_get_mac_address()), and then later found that there were generic
> "device" helpers for this, which can assist with other sorts of
> firmware nodes. It was easier to put a name on a device tree patch
> than on a "device" patch. I suppose "system firmware" might be a
> better description?
>
>>> device tree), especially in the case where the device doesn't have a
>>> useful EEPROM on which to store its MAC address (e.g., for integrated
>>> Wifi).
>>>
>>> Use the generic device helper to retrieve the MAC address, and (if
>>> present) honor it above the MAC address advertised by the card.
>>
>> But this put me back on track ;-)
>
> Let me know if you have a better way to clarify. I can resend with a
> slightly modified subject (s/firmware/system firmware/), or let Kalle
> do it, if that's the only thing to change.

"system firmware" substitution works for me.

Regards,
Arend


2018-08-28 14:34:39

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath10k: retrieve MAC address from firmware if provided

Arend van Spriel <[email protected]> writes:

> On 8/13/2018 7:14 PM, Brian Norris wrote:
>> On Sat, Aug 11, 2018 at 11:26 AM Arend van Spriel
>> <[email protected]> wrote:
>>>
>>> On 8/11/2018 1:39 AM, Brian Norris wrote:
>>>> Devices may provide their own MAC address via system firmware (e.g.,
>>>
>>> You got me confused by using just "firmware" in the subject.
>>
>> Yeah...I started by writing this patch with device tree specifically
>> (of_get_mac_address()), and then later found that there were generic
>> "device" helpers for this, which can assist with other sorts of
>> firmware nodes. It was easier to put a name on a device tree patch
>> than on a "device" patch. I suppose "system firmware" might be a
>> better description?
>>
>>>> device tree), especially in the case where the device doesn't have a
>>>> useful EEPROM on which to store its MAC address (e.g., for integrated
>>>> Wifi).
>>>>
>>>> Use the generic device helper to retrieve the MAC address, and (if
>>>> present) honor it above the MAC address advertised by the card.
>>>
>>> But this put me back on track ;-)
>>
>> Let me know if you have a better way to clarify. I can resend with a
>> slightly modified subject (s/firmware/system firmware/), or let Kalle
>> do it, if that's the only thing to change.
>
> "system firmware" substitution works for me.

What about:

ath10k: retrieve MAC address from Device Tree if provided

Because from ath10k point of view we use Device Tree functions and don't
care if it's delivered by pidgeons or by system firmware :)

I can change this before I commit.

--
Kalle Valo

2018-08-29 00:16:20

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH] ath10k: retrieve MAC address from firmware if provided

On Tue, Aug 28, 2018 at 05:33:01PM +0300, Kalle Valo wrote:
> Arend van Spriel <[email protected]> writes:
>
> > On 8/13/2018 7:14 PM, Brian Norris wrote:
> >> On Sat, Aug 11, 2018 at 11:26 AM Arend van Spriel
> >> <[email protected]> wrote:
> >>>
> >>> On 8/11/2018 1:39 AM, Brian Norris wrote:
> >>>> Devices may provide their own MAC address via system firmware (e.g.,
> >>>
> >>> You got me confused by using just "firmware" in the subject.
> >>
> >> Yeah...I started by writing this patch with device tree specifically
> >> (of_get_mac_address()), and then later found that there were generic
> >> "device" helpers for this, which can assist with other sorts of
> >> firmware nodes. It was easier to put a name on a device tree patch
> >> than on a "device" patch. I suppose "system firmware" might be a
> >> better description?
> >>
> >>>> device tree), especially in the case where the device doesn't have a
> >>>> useful EEPROM on which to store its MAC address (e.g., for integrated
> >>>> Wifi).
> >>>>
> >>>> Use the generic device helper to retrieve the MAC address, and (if
> >>>> present) honor it above the MAC address advertised by the card.
> >>>
> >>> But this put me back on track ;-)
> >>
> >> Let me know if you have a better way to clarify. I can resend with a
> >> slightly modified subject (s/firmware/system firmware/), or let Kalle
> >> do it, if that's the only thing to change.
> >
> > "system firmware" substitution works for me.
>
> What about:
>
> ath10k: retrieve MAC address from Device Tree if provided
>
> Because from ath10k point of view we use Device Tree functions and don't
> care if it's delivered by pidgeons or by system firmware :)

I don't care too much, but note that Device Tree is a loaded term,
usually referring specifically to a method of describing system hardware
via the Flattened Device Tree format. If I were specifically targeting
Device Tree, I'd use helpers like of_get_mac_address() instead. (The
'of_*' prefix is a relic of OpenFirmware, an early firmware
implementation that used the Device Tree format.)

If you're trying to say "device tree" to refer to "the Linux device
hierarchy", then that's also a fair description.

But that's all starting to mince words. Device Tree (with or without
capitalization) is fine with me.

Thanks,
Brian

> I can change this before I commit.
>
> --
> Kalle Valo

2018-09-03 16:51:25

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath10k: retrieve MAC address from firmware if provided

Brian Norris <[email protected]> writes:

> On Tue, Aug 28, 2018 at 05:33:01PM +0300, Kalle Valo wrote:
>> Arend van Spriel <[email protected]> writes:
>>
>> > On 8/13/2018 7:14 PM, Brian Norris wrote:
>> >> On Sat, Aug 11, 2018 at 11:26 AM Arend van Spriel
>> >> <[email protected]> wrote:
>> >>>
>> >>> On 8/11/2018 1:39 AM, Brian Norris wrote:
>> >>>> Devices may provide their own MAC address via system firmware (e.g.,
>> >>>
>> >>> You got me confused by using just "firmware" in the subject.
>> >>
>> >> Yeah...I started by writing this patch with device tree specifically
>> >> (of_get_mac_address()), and then later found that there were generic
>> >> "device" helpers for this, which can assist with other sorts of
>> >> firmware nodes. It was easier to put a name on a device tree patch
>> >> than on a "device" patch. I suppose "system firmware" might be a
>> >> better description?
>> >>
>> >>>> device tree), especially in the case where the device doesn't have a
>> >>>> useful EEPROM on which to store its MAC address (e.g., for integrated
>> >>>> Wifi).
>> >>>>
>> >>>> Use the generic device helper to retrieve the MAC address, and (if
>> >>>> present) honor it above the MAC address advertised by the card.
>> >>>
>> >>> But this put me back on track ;-)
>> >>
>> >> Let me know if you have a better way to clarify. I can resend with a
>> >> slightly modified subject (s/firmware/system firmware/), or let Kalle
>> >> do it, if that's the only thing to change.
>> >
>> > "system firmware" substitution works for me.
>>
>> What about:
>>
>> ath10k: retrieve MAC address from Device Tree if provided
>>
>> Because from ath10k point of view we use Device Tree functions and don't
>> care if it's delivered by pidgeons or by system firmware :)
>
> I don't care too much, but note that Device Tree is a loaded term,
> usually referring specifically to a method of describing system hardware
> via the Flattened Device Tree format. If I were specifically targeting
> Device Tree, I'd use helpers like of_get_mac_address() instead. (The
> 'of_*' prefix is a relic of OpenFirmware, an early firmware
> implementation that used the Device Tree format.)

My bad, I read your patch hastily and somehow understood you were using
of_get_mac_address() but you were actually using
device_get_mac_address(). So I'll change this to "system firmware" as
originally suggested.

Sorry for the confusion.

--
Kalle Valo

2018-09-03 16:57:10

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath10k: retrieve MAC address from firmware if provided

Brian Norris <[email protected]> wrote:

> Devices may provide their own MAC address via system firmware (e.g.,
> device tree), especially in the case where the device doesn't have a
> useful EEPROM on which to store its MAC address (e.g., for integrated
> Wifi).
>
> Use the generic device helper to retrieve the MAC address, and (if
> present) honor it above the MAC address advertised by the card.
>
> Signed-off-by: Brian Norris <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

9d5804662ce1 ath10k: retrieve MAC address from system firmware if provided

--
https://patchwork.kernel.org/patch/10563221/

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