| Message ID | 20180810233948.144792-1-briannorris@chromium.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9d5804662ce1f9bdde0a14c3c40940acbbf09538 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath10k: retrieve MAC address from firmware if provided | expand |
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 <briannorris@chromium.org> > --- > drivers/net/wireless/ath/ath10k/core.c | 3 +++ > drivers/net/wireless/ath/ath10k/wmi.c | 3 ++- > 2 files changed, 5 insertions(+), 1 deletion(-)
On Sat, Aug 11, 2018 at 11:26 AM Arend van Spriel <arend.vanspriel@broadcom.com> 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
On 8/13/2018 7:14 PM, Brian Norris wrote: > On Sat, Aug 11, 2018 at 11:26 AM Arend van Spriel > <arend.vanspriel@broadcom.com> 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
Arend van Spriel <arend.vanspriel@broadcom.com> writes: > On 8/13/2018 7:14 PM, Brian Norris wrote: >> On Sat, Aug 11, 2018 at 11:26 AM Arend van Spriel >> <arend.vanspriel@broadcom.com> 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.
On Tue, Aug 28, 2018 at 05:33:01PM +0300, Kalle Valo wrote: > Arend van Spriel <arend.vanspriel@broadcom.com> writes: > > > On 8/13/2018 7:14 PM, Brian Norris wrote: > >> On Sat, Aug 11, 2018 at 11:26 AM Arend van Spriel > >> <arend.vanspriel@broadcom.com> 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
Brian Norris <briannorris@chromium.org> writes: > On Tue, Aug 28, 2018 at 05:33:01PM +0300, Kalle Valo wrote: >> Arend van Spriel <arend.vanspriel@broadcom.com> writes: >> >> > On 8/13/2018 7:14 PM, Brian Norris wrote: >> >> On Sat, Aug 11, 2018 at 11:26 AM Arend van Spriel >> >> <arend.vanspriel@broadcom.com> 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.
Brian Norris <briannorris@chromium.org> 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 <briannorris@chromium.org> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Patch applied to ath-next branch of ath.git, thanks. 9d5804662ce1 ath10k: retrieve MAC address from system firmware if provided
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; }
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 <briannorris@chromium.org> --- drivers/net/wireless/ath/ath10k/core.c | 3 +++ drivers/net/wireless/ath/ath10k/wmi.c | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-)