[PATCH v2] rt2x00: add support for mac addr from device tree

[PATCH v2] rt2x00: add support for mac addr from device tree

[Mathias Kresin]

On some devices the EEPROMs of Ralink Wi-Fi chips have a default Ralink
MAC address set (RT3062F: 00:0C:43:30:62:00, RT3060F:
00:0C:43:30:60:00). Using multiple of these devices in the same network
can cause nasty issues.

Allow to override the MAC in the EEPROM with (a known good) one set in
the device tree to bypass the issue.

Signed-off-by: Mathias Kresin <dev@kresin.me>
---

As discussed before, forcing a random MAC for known default MACs would
be a wonky approach. I wouldn't be surprise to see ODMs setting an ODM
specific default MAC in the EEPROM. This would require a permanent
update of the list of known default MACs.

Changes in v2:

- new commit message, the former one was incomprehensible

 drivers/net/wireless/ralink/rt2x00/rt2400pci.c |  5 +----
 drivers/net/wireless/ralink/rt2x00/rt2500pci.c |  5 +----
 drivers/net/wireless/ralink/rt2x00/rt2500usb.c |  5 +----
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c |  5 +----
 drivers/net/wireless/ralink/rt2x00/rt2x00.h    |  1 +
 drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 17 +++++++++++++++++
 drivers/net/wireless/ralink/rt2x00/rt61pci.c   |  5 +----
 drivers/net/wireless/ralink/rt2x00/rt73usb.c   |  5 +----
 8 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
index 155f343..085c5b4 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
@@ -1459,10 +1459,7 @@ static int rt2400pci_validate_eeprom(struct rt2x00_dev *rt2x00dev)
 	 * Start validation of the data that has been read.
 	 */
 	mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
-	if (!is_valid_ether_addr(mac)) {
-		eth_random_addr(mac);
-		rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
-	}
+	rt2x00lib_set_mac_address(rt2x00dev, mac);
 
 	rt2x00_eeprom_read(rt2x00dev, EEPROM_ANTENNA, &word);
 	if (word == 0xffff) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
index 2553cdd..9832fd5 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
@@ -1585,10 +1585,7 @@ static int rt2500pci_validate_eeprom(struct rt2x00_dev *rt2x00dev)
 	 * Start validation of the data that has been read.
 	 */
 	mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
-	if (!is_valid_ether_addr(mac)) {
-		eth_random_addr(mac);
-		rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
-	}
+	rt2x00lib_set_mac_address(rt2x00dev, mac);
 
 	rt2x00_eeprom_read(rt2x00dev, EEPROM_ANTENNA, &word);
 	if (word == 0xffff) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
index 2d64611..cd3ab5a 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
@@ -1349,10 +1349,7 @@ static int rt2500usb_validate_eeprom(struct rt2x00_dev *rt2x00dev)
 	 * Start validation of the data that has been read.
 	 */
 	mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
-	if (!is_valid_ether_addr(mac)) {
-		eth_random_addr(mac);
-		rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
-	}
+	rt2x00lib_set_mac_address(rt2x00dev, mac);
 
 	rt2x00_eeprom_read(rt2x00dev, EEPROM_ANTENNA, &word);
 	if (word == 0xffff) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index bf3f0a3..59c49af 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -6919,10 +6919,7 @@ static int rt2800_validate_eeprom(struct rt2x00_dev *rt2x00dev)
 	 * Start validation of the data that has been read.
 	 */
 	mac = rt2800_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
-	if (!is_valid_ether_addr(mac)) {
-		eth_random_addr(mac);
-		rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
-	}
+	rt2x00lib_set_mac_address(rt2x00dev, mac);
 
 	rt2800_eeprom_read(rt2x00dev, EEPROM_NIC_CONF0, &word);
 	if (word == 0xffff) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00.h b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
index f68d492..aa3d4cee 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
@@ -1403,6 +1403,7 @@ static inline void rt2x00debug_dump_frame(struct rt2x00_dev *rt2x00dev,
  */
 u32 rt2x00lib_get_bssidx(struct rt2x00_dev *rt2x00dev,
 			 struct ieee80211_vif *vif);
+void rt2x00lib_set_mac_address(struct rt2x00_dev *rt2x00dev, u8 *eeprom_mac_addr);
 
 /*
  * Interrupt context handlers.
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
index 4e0c565..d659250 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -26,6 +26,8 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/log2.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
 
 #include "rt2x00.h"
 #include "rt2x00lib.h"
@@ -931,6 +933,21 @@ static void rt2x00lib_rate(struct ieee80211_rate *entry,
 		entry->flags |= IEEE80211_RATE_SHORT_PREAMBLE;
 }
 
+void rt2x00lib_set_mac_address(struct rt2x00_dev *rt2x00dev, u8 *eeprom_mac_addr)
+{
+	const char *mac_addr;
+
+	mac_addr = of_get_mac_address(rt2x00dev->dev->of_node);
+	if (mac_addr)
+		ether_addr_copy(eeprom_mac_addr, mac_addr);
+
+	if (!is_valid_ether_addr(eeprom_mac_addr)) {
+		eth_random_addr(eeprom_mac_addr);
+		rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", eeprom_mac_addr);
+	}
+}
+EXPORT_SYMBOL_GPL(rt2x00lib_set_mac_address);
+
 static int rt2x00lib_probe_hw_modes(struct rt2x00_dev *rt2x00dev,
 				    struct hw_mode_spec *spec)
 {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt61pci.c b/drivers/net/wireless/ralink/rt2x00/rt61pci.c
index 03013eb..5306a3b 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt61pci.c
@@ -2413,10 +2413,7 @@ static int rt61pci_validate_eeprom(struct rt2x00_dev *rt2x00dev)
 	 * Start validation of the data that has been read.
 	 */
 	mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
-	if (!is_valid_ether_addr(mac)) {
-		eth_random_addr(mac);
-		rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
-	}
+	rt2x00lib_set_mac_address(rt2x00dev, mac);
 
 	rt2x00_eeprom_read(rt2x00dev, EEPROM_ANTENNA, &word);
 	if (word == 0xffff) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt73usb.c b/drivers/net/wireless/ralink/rt2x00/rt73usb.c
index c1397a6..1a29c4d 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt73usb.c
@@ -1766,10 +1766,7 @@ static int rt73usb_validate_eeprom(struct rt2x00_dev *rt2x00dev)
 	 * Start validation of the data that has been read.
 	 */
 	mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
-	if (!is_valid_ether_addr(mac)) {
-		eth_random_addr(mac);
-		rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
-	}
+	rt2x00lib_set_mac_address(rt2x00dev, mac);
 
 	rt2x00_eeprom_read(rt2x00dev, EEPROM_ANTENNA, &word);
 	if (word == 0xffff) {
-- 
2.7.4

Re: [PATCH v2] rt2x00: add support for mac addr from device tree

[Stanislaw Gruszka]

On Fri, Aug 26, 2016 at 09:16:53AM +0200, Mathias Kresin wrote:
> On some devices the EEPROMs of Ralink Wi-Fi chips have a default Ralink
> MAC address set (RT3062F: 00:0C:43:30:62:00, RT3060F:
> 00:0C:43:30:60:00). Using multiple of these devices in the same network
> can cause nasty issues.
> 
> Allow to override the MAC in the EEPROM with (a known good) one set in
> the device tree to bypass the issue.
> 
> Signed-off-by: Mathias Kresin <dev@kresin.me>

Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>

Re: [v2] rt2x00: add support for mac addr from device tree

[Kalle Valo]

Mathias Kresin <dev@kresin.me> writes:

> 2016-09-27 17:50 GMT+02:00 Kalle Valo <kvalo@codeaurora.org>:
>> Mathias Kresin <dev@kresin.me> wrote:
>>> On some devices the EEPROMs of Ralink Wi-Fi chips have a default Ralink
>>> MAC address set (RT3062F: 00:0C:43:30:62:00, RT3060F:
>>> 00:0C:43:30:60:00). Using multiple of these devices in the same network
>>> can cause nasty issues.
>>>
>>> Allow to override the MAC in the EEPROM with (a known good) one set in
>>> the device tree to bypass the issue.
>>>
>>> Signed-off-by: Mathias Kresin <dev@kresin.me>
>>> Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>
>>
>> I think this needs to update the devicetree binding document. Also
>> remember to CC the devicetree mailing list.
>>
>> Patch set to Changes Requested.
>>
>> --
>> https://patchwork.kernel.org/patch/9300893/
>
> Hey Kalle,
>
> was it intentional not to CC the linux-wireless list?

No, that was a bug in my script. Thanks for noticing it! Please always
report back if my script does something strange.

I'm adding now linux-wireless back.

> I thought about _adding_ a devicetree binding document before sending
> the patch. But in the end it would be an empty file, since I neither
> add any bindings nor introduce a compatible string. I'm just add
> support for the generic of_get_mac_address() devicetree property,
> which is meant to be used via a devicetree pci childnode.
>
> I grepped though the 4.7 code and found a few driver using
> of_get_mac_address() without having a devicetree binding document:
>
> - drivers/net/usb/smsc95xx.c
> - drivers/net/usb/smsc75xx.c
> - drivers/net/ethernet/marvell/sky2.c
> - drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> - drivers/net/ethernet/wiznet/w5100-spi.c
> - drivers/net/ethernet/arc/emac_main.c
>
> Just let me know if you are still the opinion that a devicetree
> binding document is required to get this patch accepted.

I'm not familiar enough device tree to really comment. Can someone from
the device tree list (CCed) help?

Full patch here:

https://patchwork.kernel.org/patch/9300893/


-- 
Kalle Valo

Re: [v2] rt2x00: add support for mac addr from device tree

[Kalle Valo]

Mathias Kresin <dev-zg6vgJgm1sizQB+pC5nmwQ@public.gmane.org> writes:

> 2016-09-27 17:50 GMT+02:00 Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>:
>> Mathias Kresin <dev-zg6vgJgm1sizQB+pC5nmwQ@public.gmane.org> wrote:
>>> On some devices the EEPROMs of Ralink Wi-Fi chips have a default Ralink
>>> MAC address set (RT3062F: 00:0C:43:30:62:00, RT3060F:
>>> 00:0C:43:30:60:00). Using multiple of these devices in the same network
>>> can cause nasty issues.
>>>
>>> Allow to override the MAC in the EEPROM with (a known good) one set in
>>> the device tree to bypass the issue.
>>>
>>> Signed-off-by: Mathias Kresin <dev-zg6vgJgm1sizQB+pC5nmwQ@public.gmane.org>
>>> Acked-by: Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>
>> I think this needs to update the devicetree binding document. Also
>> remember to CC the devicetree mailing list.
>>
>> Patch set to Changes Requested.
>>
>> --
>> https://patchwork.kernel.org/patch/9300893/
>
> Hey Kalle,
>
> was it intentional not to CC the linux-wireless list?

No, that was a bug in my script. Thanks for noticing it! Please always
report back if my script does something strange.

I'm adding now linux-wireless back.

> I thought about _adding_ a devicetree binding document before sending
> the patch. But in the end it would be an empty file, since I neither
> add any bindings nor introduce a compatible string. I'm just add
> support for the generic of_get_mac_address() devicetree property,
> which is meant to be used via a devicetree pci childnode.
>
> I grepped though the 4.7 code and found a few driver using
> of_get_mac_address() without having a devicetree binding document:
>
> - drivers/net/usb/smsc95xx.c
> - drivers/net/usb/smsc75xx.c
> - drivers/net/ethernet/marvell/sky2.c
> - drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> - drivers/net/ethernet/wiznet/w5100-spi.c
> - drivers/net/ethernet/arc/emac_main.c
>
> Just let me know if you are still the opinion that a devicetree
> binding document is required to get this patch accepted.

I'm not familiar enough device tree to really comment. Can someone from
the device tree list (CCed) help?

Full patch here:

https://patchwork.kernel.org/patch/9300893/


-- 
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v2] rt2x00: add support for mac addr from device tree

[Kalle Valo]

Kalle Valo <kvalo@codeaurora.org> writes:

>> was it intentional not to CC the linux-wireless list?
>
> No, that was a bug in my script. Thanks for noticing it! Please always
> report back if my script does something strange.

The bug is now fixed, it was just a matter of a loop being in wrong
indentation level. Python is awesome but sometimes I just hate it...

Let me know if the problem still happens. And thanks again for pointing
this out.

-- 
Kalle Valo

Re: [v2] rt2x00: add support for mac addr from device tree

[Kalle Valo]

Kalle Valo <kvalo@codeaurora.org> writes:

>> I thought about _adding_ a devicetree binding document before sending
>> the patch. But in the end it would be an empty file, since I neither
>> add any bindings nor introduce a compatible string. I'm just add
>> support for the generic of_get_mac_address() devicetree property,
>> which is meant to be used via a devicetree pci childnode.
>>
>> I grepped though the 4.7 code and found a few driver using
>> of_get_mac_address() without having a devicetree binding document:
>>
>> - drivers/net/usb/smsc95xx.c
>> - drivers/net/usb/smsc75xx.c
>> - drivers/net/ethernet/marvell/sky2.c
>> - drivers/net/ethernet/cavium/thunder/thunder_bgx.c
>> - drivers/net/ethernet/wiznet/w5100-spi.c
>> - drivers/net/ethernet/arc/emac_main.c
>>
>> Just let me know if you are still the opinion that a devicetree
>> binding document is required to get this patch accepted.
>
> I'm not familiar enough device tree to really comment. Can someone from
> the device tree list (CCed) help?
>
> Full patch here:
>
> https://patchwork.kernel.org/patch/9300893/

I didn't get any comments so I guess we don't need to document the use
of mac-address in the driver binding document. At least
Documentation/devicetree/bindings/net/ethernet.txt supports that, even
if rt2x00 is a wireless driver.

Unless I get any objections I'm planning to apply this to
wireless-drivers-next (once it's open again).

-- 
Kalle Valo

Re: [v2] rt2x00: add support for mac addr from device tree

[Kalle Valo]

Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> writes:

>> I thought about _adding_ a devicetree binding document before sending
>> the patch. But in the end it would be an empty file, since I neither
>> add any bindings nor introduce a compatible string. I'm just add
>> support for the generic of_get_mac_address() devicetree property,
>> which is meant to be used via a devicetree pci childnode.
>>
>> I grepped though the 4.7 code and found a few driver using
>> of_get_mac_address() without having a devicetree binding document:
>>
>> - drivers/net/usb/smsc95xx.c
>> - drivers/net/usb/smsc75xx.c
>> - drivers/net/ethernet/marvell/sky2.c
>> - drivers/net/ethernet/cavium/thunder/thunder_bgx.c
>> - drivers/net/ethernet/wiznet/w5100-spi.c
>> - drivers/net/ethernet/arc/emac_main.c
>>
>> Just let me know if you are still the opinion that a devicetree
>> binding document is required to get this patch accepted.
>
> I'm not familiar enough device tree to really comment. Can someone from
> the device tree list (CCed) help?
>
> Full patch here:
>
> https://patchwork.kernel.org/patch/9300893/

I didn't get any comments so I guess we don't need to document the use
of mac-address in the driver binding document. At least
Documentation/devicetree/bindings/net/ethernet.txt supports that, even
if rt2x00 is a wireless driver.

Unless I get any objections I'm planning to apply this to
wireless-drivers-next (once it's open again).

-- 
Kalle Valo

Re: [v2] rt2x00: add support for mac addr from device tree

[Kalle Valo]

Mathias Kresin <dev@kresin.me> wrote:
> On some devices the EEPROMs of Ralink Wi-Fi chips have a default Ralink
> MAC address set (RT3062F: 00:0C:43:30:62:00, RT3060F:
> 00:0C:43:30:60:00). Using multiple of these devices in the same network
> can cause nasty issues.
> 
> Allow to override the MAC in the EEPROM with (a known good) one set in
> the device tree to bypass the issue.
> 
> Signed-off-by: Mathias Kresin <dev@kresin.me>
> Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>

Patch applied to wireless-drivers-next.git, thanks.

9766cb709089 rt2x00: add support for mac addr from device tree

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

Documentation about submitting wireless patches and checking status
from patchwork:

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