Skip to content

Commit

Permalink
treewide: backport support for nvmem on non platform devices
Browse files Browse the repository at this point in the history
In the current state, nvmem cells are only detected on platform device.
To quickly fix the problem, we register the affected problematic driver
with the of_platform but that is more an hack than a real solution.
Backport from net-next the required patch so that nvmem can work also
with non-platform devices and rework our current patch.
Drop the mediatek and dsa workaround and rework the ath10k patches.
Rework every driver that use the of_get_mac_address api.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
  • Loading branch information
Ansuel authored and blocktrron committed Aug 4, 2021
1 parent edb6bc1 commit 91a52f2
Show file tree
Hide file tree
Showing 35 changed files with 4,460 additions and 500 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,19 @@ diff --git a/ath10k-5.10/core.c b/ath10k-5.10/core.c
index 5f4e12196..9ed7b9883 100644
--- a/ath10k-5.10/core.c
+++ b/ath10k-5.10/core.c
@@ -8,6 +8,9 @@
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
#include <linux/of.h>
+#include <linux/of_net.h>
+#include <linux/of_platform.h>
+#include <linux/property.h>
#include <linux/property.h>
#include <linux/dmi.h>
#include <linux/ctype.h>
@@ -2961,8 +2963,14 @@ EXPORT_SYMBOL(ath10k_core_stop);
static int ath10k_core_probe_fw(struct ath10k *ar)
{
struct bmi_target_info target_info;
+ const char *mac;
int ret = 0;

+#ifdef CONFIG_OF
+ /* register the platform to be found by the of api */
+ of_platform_device_create(ar->dev->of_node, NULL, NULL);
+#endif
+
ret = ath10k_hif_power_up(ar, ATH10K_FIRMWARE_MODE_NORMAL);
if (ret) {
ath10k_err(ar, "could not power on hif bus (%d)\n", ret);
@@ -3062,6 +3068,10 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
@@ -3062,6 +3068,8 @@ static int ath10k_core_probe_fw(struct ath10k *ar)

device_get_mac_address(ar->dev, ar->mac_addr, sizeof(ar->mac_addr));

+ mac = of_get_mac_address(ar->dev->of_node);
+ if (!IS_ERR(mac))
+ ether_addr_copy(ar->mac_addr, mac);
+ of_get_mac_address(ar->dev->of_node, ar->mac_addr);
+
ret = ath10k_core_init_firmware_features(ar);
if (ret) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,19 @@ diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/a
index 5f4e12196..9ed7b9883 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -8,6 +8,9 @@
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
#include <linux/of.h>
+#include <linux/of_net.h>
+#include <linux/of_platform.h>
+#include <linux/property.h>
#include <linux/property.h>
#include <linux/dmi.h>
#include <linux/ctype.h>
@@ -2961,8 +2963,14 @@ EXPORT_SYMBOL(ath10k_core_stop);
static int ath10k_core_probe_fw(struct ath10k *ar)
{
struct bmi_target_info target_info;
+ const char *mac;
int ret = 0;

+#ifdef CONFIG_OF
+ /* register the platform to be found by the of api */
+ of_platform_device_create(ar->dev->of_node, NULL, NULL);
+#endif
+
ret = ath10k_hif_power_up(ar, ATH10K_FIRMWARE_MODE_NORMAL);
if (ret) {
ath10k_err(ar, "could not power on hif bus (%d)\n", ret);
@@ -3062,6 +3068,10 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
@@ -3062,6 +3068,8 @@ static int ath10k_core_probe_fw(struct ath10k *ar)

device_get_mac_address(ar->dev, ar->mac_addr, sizeof(ar->mac_addr));

+ mac = of_get_mac_address(ar->dev->of_node);
+ if (!IS_ERR(mac))
+ ether_addr_copy(ar->mac_addr, mac);
+ of_get_mac_address(ar->dev->of_node, ar->mac_addr);
+
ret = ath10k_core_init_firmware_features(ar);
if (ret) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -990,8 +990,13 @@ static void rt2x00lib_rate(struct ieee80
@@ -990,6 +990,12 @@ static void rt2x00lib_rate(struct ieee80

void rt2x00lib_set_mac_address(struct rt2x00_dev *rt2x00dev, u8 *eeprom_mac_addr)
{
+ struct rt2x00_platform_data *pdata;
const char *mac_addr;

+
+ pdata = rt2x00dev->dev->platform_data;
+ if (pdata && pdata->mac_address)
+ ether_addr_copy(eeprom_mac_addr, pdata->mac_address);
+
mac_addr = of_get_mac_address(rt2x00dev->dev->of_node);
if (!IS_ERR(mac_addr))
ether_addr_copy(eeprom_mac_addr, mac_addr);
of_get_mac_address(rt2x00dev->dev->of_node, eeprom_mac_addr);

if (!is_valid_ether_addr(eeprom_mac_addr)) {
--- a/include/linux/rt2x00_platform.h
+++ b/include/linux/rt2x00_platform.h
@@ -14,6 +14,7 @@
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,245 @@
From 83216e3988cd196183542937c9bd58b279f946af Mon Sep 17 00:00:00 2001
From: Michael Walle <michael@walle.cc>
Date: Mon, 12 Apr 2021 19:47:17 +0200
Subject: of: net: pass the dst buffer to of_get_mac_address()

of_get_mac_address() returns a "const void*" pointer to a MAC address.
Lately, support to fetch the MAC address by an NVMEM provider was added.
But this will only work with platform devices. It will not work with
PCI devices (e.g. of an integrated root complex) and esp. not with DSA
ports.

There is an of_* variant of the nvmem binding which works without
devices. The returned data of a nvmem_cell_read() has to be freed after
use. On the other hand the return of_get_mac_address() points to some
static data without a lifetime. The trick for now, was to allocate a
device resource managed buffer which is then returned. This will only
work if we have an actual device.

Change it, so that the caller of of_get_mac_address() has to supply a
buffer where the MAC address is written to. Unfortunately, this will
touch all drivers which use the of_get_mac_address().

Usually the code looks like:

const char *addr;
addr = of_get_mac_address(np);
if (!IS_ERR(addr))
ether_addr_copy(ndev->dev_addr, addr);

This can then be simply rewritten as:

of_get_mac_address(np, ndev->dev_addr);

Sometimes is_valid_ether_addr() is used to test the MAC address.
of_get_mac_address() already makes sure, it just returns a valid MAC
address. Thus we can just test its return code. But we have to be
careful if there are still other sources for the MAC address before the
of_get_mac_address(). In this case we have to keep the
is_valid_ether_addr() call.

The following coccinelle patch was used to convert common cases to the
new style. Afterwards, I've manually gone over the drivers and fixed the
return code variable: either used a new one or if one was already
available use that. Mansour Moufid, thanks for that coccinelle patch!

<spml>
@a@
identifier x;
expression y, z;
@@
- x = of_get_mac_address(y);
+ x = of_get_mac_address(y, z);
<...
- ether_addr_copy(z, x);
...>

@@
identifier a.x;
@@
- if (<+... x ...+>) {}

@@
identifier a.x;
@@
if (<+... x ...+>) {
...
}
- else {}

@@
identifier a.x;
expression e;
@@
- if (<+... x ...+>@e)
- {}
- else
+ if (!(e))
{...}

@@
expression x, y, z;
@@
- x = of_get_mac_address(y, z);
+ of_get_mac_address(y, z);
... when != x
</spml>

All drivers, except drivers/net/ethernet/aeroflex/greth.c, were
compile-time tested.

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
arch/arm/mach-mvebu/kirkwood.c | 3 +-
arch/powerpc/sysdev/tsi108_dev.c | 5 +-
drivers/net/ethernet/aeroflex/greth.c | 6 +--
drivers/net/ethernet/allwinner/sun4i-emac.c | 10 ++--
drivers/net/ethernet/altera/altera_tse_main.c | 7 +--
drivers/net/ethernet/arc/emac_main.c | 8 +--
drivers/net/ethernet/atheros/ag71xx.c | 7 +--
drivers/net/ethernet/broadcom/bcm4908_enet.c | 7 +--
drivers/net/ethernet/broadcom/bcmsysport.c | 7 +--
drivers/net/ethernet/broadcom/bgmac-bcma.c | 10 ++--
drivers/net/ethernet/broadcom/bgmac-platform.c | 11 ++--
drivers/net/ethernet/cadence/macb_main.c | 11 ++--
drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 8 +--
drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 5 +-
drivers/net/ethernet/davicom/dm9000.c | 10 ++--
drivers/net/ethernet/ethoc.c | 6 +--
drivers/net/ethernet/ezchip/nps_enet.c | 7 +--
drivers/net/ethernet/freescale/fec_main.c | 7 +--
drivers/net/ethernet/freescale/fec_mpc52xx.c | 7 +--
drivers/net/ethernet/freescale/fman/mac.c | 9 ++--
.../net/ethernet/freescale/fs_enet/fs_enet-main.c | 5 +-
drivers/net/ethernet/freescale/gianfar.c | 8 +--
drivers/net/ethernet/freescale/ucc_geth.c | 5 +-
drivers/net/ethernet/hisilicon/hisi_femac.c | 7 +--
drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 7 +--
drivers/net/ethernet/lantiq_xrx200.c | 7 +--
drivers/net/ethernet/marvell/mv643xx_eth.c | 5 +-
drivers/net/ethernet/marvell/mvneta.c | 6 +--
.../net/ethernet/marvell/prestera/prestera_main.c | 11 ++--
drivers/net/ethernet/marvell/pxa168_eth.c | 9 +---
drivers/net/ethernet/marvell/sky2.c | 8 ++-
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 ++--
drivers/net/ethernet/micrel/ks8851_common.c | 7 ++-
drivers/net/ethernet/microchip/lan743x_main.c | 5 +-
drivers/net/ethernet/nxp/lpc_eth.c | 4 +-
drivers/net/ethernet/qualcomm/qca_spi.c | 10 ++--
drivers/net/ethernet/qualcomm/qca_uart.c | 9 +---
drivers/net/ethernet/renesas/ravb_main.c | 12 +++--
drivers/net/ethernet/renesas/sh_eth.c | 5 +-
.../net/ethernet/samsung/sxgbe/sxgbe_platform.c | 13 ++---
drivers/net/ethernet/socionext/sni_ave.c | 10 ++--
.../net/ethernet/stmicro/stmmac/dwmac-anarion.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-generic.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-intel-plat.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-ipq806x.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-mediatek.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-visconti.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 14 ++---
.../net/ethernet/stmicro/stmmac/stmmac_platform.h | 2 +-
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 ++++---
drivers/net/ethernet/ti/cpsw.c | 7 +--
drivers/net/ethernet/ti/cpsw_new.c | 7 +--
drivers/net/ethernet/ti/davinci_emac.c | 8 +--
drivers/net/ethernet/ti/netcp_core.c | 7 +--
drivers/net/ethernet/wiznet/w5100-spi.c | 8 ++-
drivers/net/ethernet/wiznet/w5100.c | 2 +-
drivers/net/ethernet/xilinx/ll_temac_main.c | 8 +--
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 15 +++---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 8 +--
drivers/net/wireless/ath/ath9k/init.c | 5 +-
drivers/net/wireless/mediatek/mt76/eeprom.c | 9 +---
drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 6 +--
drivers/of/of_net.c | 60 ++++++++++------------
drivers/staging/octeon/ethernet.c | 10 ++--
drivers/staging/wfx/main.c | 7 ++-
include/linux/of_net.h | 6 +--
include/net/dsa.h | 2 +-
net/dsa/dsa2.c | 2 +-
net/dsa/slave.c | 2 +-
net/ethernet/eth.c | 11 ++--
85 files changed, 218 insertions(+), 364 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 01f9c26f9bf37..e9a36dd7144f1 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -617,7 +617,6 @@ static int ath9k_of_init(struct ath_softc *sc)
struct ath_hw *ah = sc->sc_ah;
struct ath_common *common = ath9k_hw_common(ah);
enum ath_bus_type bus_type = common->bus_ops->ath_bus_type;
- const char *mac;
char eeprom_name[100];
int ret;

@@ -640,9 +639,7 @@ static int ath9k_of_init(struct ath_softc *sc)
ah->ah_flags |= AH_NO_EEP_SWAP;
}

- mac = of_get_mac_address(np);
- if (!IS_ERR(mac))
- ether_addr_copy(common->macaddr, mac);
+ of_get_mac_address(np, common->macaddr);

return 0;
}
diff --git a/drivers/net/wireless/mediatek/mt76/eeprom.c b/drivers/net/wireless/mediatek/mt76/eeprom.c
index 665b54c5c8ae5..6d895738222ad 100644
--- a/drivers/net/wireless/mediatek/mt76/eeprom.c
+++ b/drivers/net/wireless/mediatek/mt76/eeprom.c
@@ -91,15 +91,9 @@ void
void
mt76_eeprom_override(struct mt76_dev *dev)
{
-#ifdef CONFIG_OF
struct device_node *np = dev->dev->of_node;
- const u8 *mac = NULL;

- if (np)
- mac = of_get_mac_address(np);
- if (!IS_ERR_OR_NULL(mac))
- ether_addr_copy(dev->macaddr, mac);
-#endif
+ of_get_mac_address(np, dev->macaddr);

if (!is_valid_ether_addr(dev->macaddr)) {
eth_random_addr(dev->macaddr);
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
index 61a4f1ad31e28..e95c101c27111 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -989,11 +989,7 @@ static void rt2x00lib_rate(struct ieee80211_rate *entry,

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 (!IS_ERR(mac_addr))
- ether_addr_copy(eeprom_mac_addr, mac_addr);
+ of_get_mac_address(rt2x00dev->dev->of_node, eeprom_mac_addr);

if (!is_valid_ether_addr(eeprom_mac_addr)) {
eth_random_addr(eeprom_mac_addr);
--
cgit 1.2.3-1.el7

14 comments on commit 91a52f2

@hnyman
Copy link
Contributor

@hnyman hnyman commented on 91a52f2 Aug 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ansuel @blocktrron

I think that this commit has causes a lot of breakage in buildbot. Most targets seem to be broken.

Looks like drivers/net/usb/smsc95xx.c is still unpatched???

Looking back at the buildbot error logs, this gets shown up in every failing target:

x86/geode

drivers/net/usb/smsc95xx.c: In function 'smsc95xx_init_mac_address':
drivers/net/usb/smsc95xx.c:907:13: error: too few arguments to function 'of_get_mac_address'
  mac_addr = of_get_mac_address(dev->udev->dev.of_node);
             ^~~~~~~~~~~~~~~~~~
In file included from drivers/net/usb/smsc95xx.c:20:
./include/linux/of_net.h:22:19: note: declared here
 static inline int of_get_mac_address(struct device_node *np, u8 *mac)
                   ^~~~~~~~~~~~~~~~~~
  CC      drivers/usb/host/xhci-mem.o
  CC      drivers/scsi/scsi_transport_spi.o
  CC [M]  drivers/staging/rtl8712/rtl8712_led.o
  AR      net/netfilter/built-in.a
make[7]: *** [scripts/Makefile.build:262: drivers/net/usb/smsc95xx.o] Error 1
make[7]: *** Waiting for unfinished jobs....

armvirt32:

drivers/net/usb/smsc95xx.c: In function 'smsc95xx_init_mac_address':
drivers/net/usb/smsc95xx.c:907:13: error: too few arguments to function 'of_get_mac_address'
  mac_addr = of_get_mac_address(dev->udev->dev.of_node);
             ^~~~~~~~~~~~~~~~~~
In file included from drivers/net/usb/smsc95xx.c:20:
./include/linux/of_net.h:14:12: note: declared here
 extern int of_get_mac_address(struct device_node *np, u8 *mac);
            ^~~~~~~~~~~~~~~~~~
make[7]: *** [scripts/Makefile.build:262: drivers/net/usb/smsc95xx.o] Error 1
make[7]: *** Waiting for unfinished jobs....

@fda77
Copy link

@fda77 fda77 commented on 91a52f2 Aug 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes with avm7320:

[    0.675430] Kernel bug detected[#1]:
[    0.678548] CPU: 0 PID: 1 Comm: swapper Not tainted 5.4.137 #0
[    0.684314] $ 0   : 00000000 00000001 ffffffff 8086eacc
[    0.689488] $ 4   : 8085a860 00000000 00000000 00000000
[    0.694663] $ 8   : 806823f8 805743a8 83d00038 00000000
[    0.699839] $12   : 00000000 00000000 00000001 00000000
[    0.705015] $16   : 8085a860 80670000 8086eac8 00000000
[    0.710191] $20   : 83c2dcb0 806e55bc 00000000 80820000
[    0.715366] $24   : 00000000 00000000
[    0.720542] $28   : 83c2c000 83c2dc50 80712560 801252e8
[    0.725719] Hi    : 00000043
[    0.728566] Lo    : 3ae148bb
[    0.731438] epc   : 8012443c BUG+0x0/0x4
[    0.735307] ra    : 801252e8 kmem_cache_free_bulk+0x0/0x550
[    0.740815] Status: 1100fc03 KERNEL EXL IE
[    0.744958] Cause : 10800024 (ExcCode 09)
[    0.748926] PrId  : 0001954c (MIPS 34Kc)
[    0.752804] Modules linked in:
[    0.755833] Process swapper (pid: 1, threadinfo=(ptrval), task=(ptrval), tls=00000000)
[    0.763673] Stack : 80680000 00000000 00000001 80670000 83cb6210 803d7a50 00000001 8085a74c
[    0.771954]         80670000 83cb6210 83d00898 803e033c 83d74f60 80409754 8085a74c 803dc0d0
[    0.780235]         00000000 00000200 83cb6210 83cb6200 8085a74c 00000005 83cb6210 803e0114
[    0.788516]         fffffffe 80448ce4 00000000 83d00000 83cb6200 83cb6210 83d7a880 80600000
[    0.796797]         806e55bc 803aa384 83d7a480 83c28000 00000000 83d79de0 83cb5400 83cb3cc0
[    0.805079]         ...
[    0.807496] Call Trace:
[    0.809922] [<8012443c>] BUG+0x0/0x4
[    0.813463] [<801252e8>] kmem_cache_free_bulk+0x0/0x550
[    0.818626] Code: 00000000  1000ffda  24021000 <000c000d> 00a01025  8c8600d0  3c058061  24a5fcb8  0815c023
[    0.828284]
[    0.829855] ---[ end trace 454b72dd0400f3d7 ]---
[    0.834370] Kernel panic - not syncing: Fatal exception

@ptpt52
Copy link
Contributor

@ptpt52 ptpt52 commented on 91a52f2 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ansuel @blocktrron @hnyman

hi

I don't know what issue it is but it is really not work on some devices:
known issue on device:

netgear,ex7300
qihoo,c301

and more device

but there is no issue on nand flash device

someone report bug here https://forum.openwrt.org/t/adding-openwrt-support-for-tplink-tl-wr841hp-v2/69445/52

[    0.550696] ag71xx 19000000.eth: invalid MAC address, using random address
[    0.900659] ag71xx 1a000000.eth: invalid MAC address, using random address
[    1.810536] ag71xx 19000000.eth: invalid MAC address, using random address

@Ansuel
Copy link
Member Author

@Ansuel Ansuel commented on 91a52f2 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ptpt52 can you put here a link to a dts file ?

@ptpt52
Copy link
Contributor

@ptpt52 ptpt52 commented on 91a52f2 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ptpt52 can you put here a link to a dts file ?

@Ansuel sure, dts file like this one:
https://github.com/openwrt/openwrt/blob/master/target/linux/ath79/dts/qca9558_netgear_ex7300.dtsi

@Ansuel
Copy link
Member Author

@Ansuel Ansuel commented on 91a52f2 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the history of this device it never had a mtd-mac-address and it wasn't converted so how the mac address was set before this change? I assume it wasn't ?

@ptpt52
Copy link
Contributor

@ptpt52 ptpt52 commented on 91a52f2 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the history of this device it never had a mtd-mac-address and it wasn't converted so how the mac address was set before this change? I assume it wasn't ?

sorry, this one

https://github.com/openwrt/openwrt/blob/master/target/linux/ath79/dts/qca9558_netgear_ex7300.dtsi

OR

https://github.com/x-wrt/x-wrt/blob/master/target/linux/ath79/dts/ar9344_bm100_hq55.dts

@Ansuel
Copy link
Member Author

@Ansuel Ansuel commented on 91a52f2 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean it could be a problem with nvmem deferring probe... and we don't handle that... The patch should very easy (if the driver can be deferred)
(we should check the return of of_get_mac_address and check if the error is -EPROBE_DEFER and return that.

@ptpt52
Copy link
Contributor

@ptpt52 ptpt52 commented on 91a52f2 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ptpt52
Copy link
Contributor

@ptpt52 ptpt52 commented on 91a52f2 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may post some boot log on my hq55 device:

[    0.000000] Linux version 5.4.167 (ptpt52@SC-GAME) (gcc version 11.2.0 (OpenWrt GCC 11.2.0 r18360+1-4d78c0c958)) #0 Sat Dec 18 09:31:35 2021
[    0.000000] printk: bootconsole [early0] enabled
[    0.000000] CPU0 revision is: 0001974c (MIPS 74Kc)
[    0.000000] MIPS: machine is BM100 HQ55
[    0.000000] SoC: Atheros AR9344 rev 2
[    0.000000] Initrd not found or empty - disabling initrd
[    0.000000] Primary instruction cache 64kB, VIPT, 4-way, linesize 32 bytes.
[    0.000000] Primary data cache 32kB, 4-way, VIPT, cache aliases, linesize 32 bytes
[    0.000000] Zone ranges:
[    0.000000]   Normal   [mem 0x0000000000000000-0x0000000007ffffff]
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000000000000-0x0000000007ffffff]
[    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x0000000007ffffff]
[    0.000000] On node 0 totalpages: 32768
[    0.000000]   Normal zone: 256 pages used for memmap
[    0.000000]   Normal zone: 0 pages reserved
[    0.000000]   Normal zone: 32768 pages, LIFO batch:7
[    0.000000] pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
[    0.000000] pcpu-alloc: [0] 0 
[    0.000000] Built 1 zonelists, mobility grouping on.  Total pages: 32512
[    0.000000] Kernel command line: console=ttyS0,115200n8 rootfstype=squashfs,jffs2
[    0.000000] Dentry cache hash table entries: 16384 (order: 4, 65536 bytes, linear)
[    0.000000] Inode-cache hash table entries: 8192 (order: 3, 32768 bytes, linear)
[    0.000000] Writing ErrCtl register=00000000
[    0.000000] Readback ErrCtl register=00000000
[    0.000000] mem auto-init: stack:off, heap alloc:off, heap free:off
[    0.000000] Memory: 121744K/131072K available (4904K kernel code, 166K rwdata, 1108K rodata, 1236K init, 193K bss, 9328K reserved, 0K cma-reserved)
[    0.000000] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[    0.000000] NR_IRQS: 51
[    0.000000] random: get_random_bytes called from start_kernel+0x34c/0x520 with crng_init=0
[    0.000000] CPU clock: 550.000 MHz
[    0.000000] clocksource: MIPS: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 6950037990 ns
[    0.000009] sched_clock: 32 bits at 275MHz, resolution 3ns, wraps every 7809031678ns
[    0.008701] Calibrating delay loop... 274.02 BogoMIPS (lpj=1370112)
[    0.085645] pid_max: default: 32768 minimum: 301
[    0.090965] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes, linear)
[    0.099125] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes, linear)
[    0.109851] devtmpfs: initialized
[    0.116417] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[    0.127428] futex hash table entries: 256 (order: -1, 3072 bytes, linear)
[    0.135061] pinctrl core: initialized pinctrl subsystem
[    0.141967] NET: Registered protocol family 16
[    0.153878] pstore: Registered ramoops as persistent store backend
[    0.160768] ramoops: using 0x80000@0x3f00000, ecc: 0
[    0.197262] clocksource: Switched to clocksource MIPS
[    0.206691] NET: Registered protocol family 2
[    0.211752] IP idents hash table entries: 2048 (order: 2, 16384 bytes, linear)
[    0.220522] tcp_listen_portaddr_hash hash table entries: 512 (order: 0, 4096 bytes, linear)
[    0.229909] TCP established hash table entries: 1024 (order: 0, 4096 bytes, linear)
[    0.238475] TCP bind hash table entries: 1024 (order: 0, 4096 bytes, linear)
[    0.246344] TCP: Hash tables configured (established 1024 bind 1024)
[    0.253583] UDP hash table entries: 256 (order: 0, 4096 bytes, linear)
[    0.260929] UDP-Lite hash table entries: 256 (order: 0, 4096 bytes, linear)
[    0.269007] NET: Registered protocol family 1
[    0.273903] PCI: CLS 0 bytes, default 32
[    0.281383] workingset: timestamp_bits=30 max_order=15 bucket_order=0
[    0.294824] squashfs: version 4.0 (2009/01/31) Phillip Lougher
[    0.301392] jffs2: version 2.2 (NAND) (SUMMARY) (LZMA) (RTIME) (CMODE_PRIORITY) (c) 2001-2006 Red Hat, Inc.
[    0.326545] pinctrl-single 1804002c.pinmux: 544 pins, size 68
[    0.337431] Serial: 8250/16550 driver, 1 ports, IRQ sharing disabled
[    0.346325] printk: console [ttyS0] disabled
[    0.351212] 18020000.uart: ttyS0 at MMIO 0x18020000 (irq = 9, base_baud = 2500000) is a 16550A
[    0.360857] printk: console [ttyS0] enabled
[    0.369761] printk: bootconsole [early0] disabled
[    0.384426] spi-nor spi0.0: mx25l12805d (16384 Kbytes)
[    0.389769] 8 fixed-partitions partitions found on MTD device spi0.0
[    0.396208] Creating 8 MTD partitions on "spi0.0":
[    0.401105] 0x000000000000-0x000000040000 : "u-boot"
[    0.408806] 0x000000040000-0x000000050000 : "u-boot-env"
[    0.416534] 0x000000050000-0x000000060000 : "loader1"
[    0.424165] 0x000000060000-0x000000eb0000 : "firmware"
[    0.432144] 2 uimage-fw partitions found on MTD device firmware
[    0.438221] Creating 2 MTD partitions on "firmware":
[    0.443263] 0x000000000000-0x000000200000 : "kernel"
[    0.450542] 0x000000200000-0x000000e50000 : "rootfs"
[    0.457929] mtd: device 5 (rootfs) set to be root filesystem
[    0.464371] 1 squashfs-split partitions found on MTD device rootfs
[    0.470727] 0x000000c00000-0x000000e50000 : "rootfs_data"
[    0.478068] 0x000000eb0000-0x000000ec0000 : "loader2"
[    0.485588] 0x000000ec0000-0x000000ff0000 : "unused"
[    0.493039] 0x000000050000-0x000000ff0000 : "oem-firmware"
[    0.501082] 0x000000ff0000-0x000001000000 : "art"
[    0.510420] libphy: Fixed MDIO Bus: probed
[    0.518350] of_get_mac_addr_nvmem:68 ret=-2
[    0.522612] of_get_mac_addr_nvmem:75 ret=-2
[    0.526861] ag71xx 19000000.eth: invalid MAC address, using random address
[    0.868376] ag71xx 19000000.eth: Could not connect to PHY device. Deferring probe.
[    0.876431] of_get_mac_addr_nvmem:68 ret=-2
[    0.880733] of_get_mac_addr_nvmem:75 ret=-2
[    0.884985] ag71xx 1a000000.eth: invalid MAC address, using random address
[    1.567677] libphy: ag71xx_mdio: probed
[    1.572832] libphy: ar8xxx-mdio: probed
[    1.584279] switch0: Atheros AR8229 rev. 1 switch registered on mdio.0
[    1.636906] ag71xx 1a000000.eth: connected to PHY at fixed-0:00 [uid=00000000, driver=Generic PHY]
[    1.646719] eth0: Atheros AG71xx at 0xba000000, irq 5, mode: gmii
[    1.653193] i2c /dev entries driver
[    1.661362] NET: Registered protocol family 10
[    1.670509] Segment Routing with IPv6
[    1.674353] NET: Registered protocol family 17
[    1.678999] bridge: filtering via arp/ip/ip6tables is no longer available by default. Update your scripts to load br_netfilter if you need this.
[    1.692160] 8021q: 802.1Q VLAN Support v1.8
[    1.698021] pstore: Using crash dump compression: deflate
[    1.704884] of_get_mac_addr_nvmem:68 ret=-2
[    1.709199] of_get_mac_addr_nvmem:75 ret=-2
[    1.713454] ag71xx 19000000.eth: invalid MAC address, using random address
[    2.059178] ag71xx 19000000.eth: connected to PHY at mdio.0:1f:04 [uid=004dd042, driver=Generic PHY]
[    2.069485] eth1: Atheros AG71xx at 0xb9000000, irq 4, mode: mii
[    2.082305] VFS: Mounted root (squashfs filesystem) readonly on device 31:5.
[    2.094492] devtmpfs: mounted
[    2.104305] Freeing unused kernel memory: 1236K
[    2.108937] This architecture does not have kernel memory protection.
[    2.115464] Run /sbin/init as init process
[    2.547276] random: fast init done
[    2.935139] init: Console is alive
[    2.939186] init: - watchdog -

@ptpt52
Copy link
Contributor

@ptpt52 ptpt52 commented on 91a52f2 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean it could be a problem with nvmem deferring probe... and we don't handle that... The patch should very easy (if the driver can be deferred) (we should check the return of of_get_mac_address and check if the error is -EPROBE_DEFER and return that.

ok this may be right, I may try this:

diff --git a/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c b/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
index a38b336a8d..c456cc0262 100644
--- a/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
+++ b/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
@@ -1673,12 +1673,6 @@ static int ag71xx_probe(struct platform_device *pdev)
        ag->stop_desc->ctrl = 0;
        ag->stop_desc->next = (u32) ag->stop_desc_dma;
 
-       of_get_mac_address(np, dev->dev_addr);
-       if (!is_valid_ether_addr(dev->dev_addr)) {
-               dev_err(&pdev->dev, "invalid MAC address, using random address\n");
-               eth_random_addr(dev->dev_addr);
-       }
-
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(5,10,0)
        err = of_get_phy_mode(np, &ag->phy_if_mode);
        if (err < 0) {
@@ -1750,6 +1744,12 @@ static int ag71xx_probe(struct platform_device *pdev)
                goto err_phy_disconnect;
        }
 
+       of_get_mac_address(np, dev->dev_addr);
+       if (!is_valid_ether_addr(dev->dev_addr)) {
+               dev_err(&pdev->dev, "invalid MAC address, using random address\n");
+               eth_random_addr(dev->dev_addr);
+       }
+
        pr_info("%s: Atheros AG71xx at 0x%08lx, irq %d, mode: %s\n",
                dev->name, (unsigned long) ag->mac_base, dev->irq,
                phy_modes(ag->phy_if_mode));

@ptpt52
Copy link
Contributor

@ptpt52 ptpt52 commented on 91a52f2 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean it could be a problem with nvmem deferring probe... and we don't handle that... The patch should very easy (if the driver can be deferred) (we should check the return of of_get_mac_address and check if the error is -EPROBE_DEFER and return that.

ok this may be right, I may try this:

diff --git a/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c b/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
index a38b336a8d..c456cc0262 100644
--- a/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
+++ b/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
@@ -1673,12 +1673,6 @@ static int ag71xx_probe(struct platform_device *pdev)
        ag->stop_desc->ctrl = 0;
        ag->stop_desc->next = (u32) ag->stop_desc_dma;
 
-       of_get_mac_address(np, dev->dev_addr);
-       if (!is_valid_ether_addr(dev->dev_addr)) {
-               dev_err(&pdev->dev, "invalid MAC address, using random address\n");
-               eth_random_addr(dev->dev_addr);
-       }
-
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(5,10,0)
        err = of_get_phy_mode(np, &ag->phy_if_mode);
        if (err < 0) {
@@ -1750,6 +1744,12 @@ static int ag71xx_probe(struct platform_device *pdev)
                goto err_phy_disconnect;
        }
 
+       of_get_mac_address(np, dev->dev_addr);
+       if (!is_valid_ether_addr(dev->dev_addr)) {
+               dev_err(&pdev->dev, "invalid MAC address, using random address\n");
+               eth_random_addr(dev->dev_addr);
+       }
+
        pr_info("%s: Atheros AG71xx at 0x%08lx, irq %d, mode: %s\n",
                dev->name, (unsigned long) ag->mac_base, dev->irq,
                phy_modes(ag->phy_if_mode));

tested
no help here.

I have to revert some code to use the mtd-mac-address implementation. and it works.

@Ansuel
Copy link
Member Author

@Ansuel Ansuel commented on 91a52f2 Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean it could be a problem with nvmem deferring probe... and we don't handle that... The patch should very easy (if the driver can be deferred) (we should check the return of of_get_mac_address and check if the error is -EPROBE_DEFER and return that.

ok this may be right, I may try this:

diff --git a/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c b/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
index a38b336a8d..c456cc0262 100644
--- a/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
+++ b/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
@@ -1673,12 +1673,6 @@ static int ag71xx_probe(struct platform_device *pdev)
        ag->stop_desc->ctrl = 0;
        ag->stop_desc->next = (u32) ag->stop_desc_dma;
 
-       of_get_mac_address(np, dev->dev_addr);
-       if (!is_valid_ether_addr(dev->dev_addr)) {
-               dev_err(&pdev->dev, "invalid MAC address, using random address\n");
-               eth_random_addr(dev->dev_addr);
-       }
-
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(5,10,0)
        err = of_get_phy_mode(np, &ag->phy_if_mode);
        if (err < 0) {
@@ -1750,6 +1744,12 @@ static int ag71xx_probe(struct platform_device *pdev)
                goto err_phy_disconnect;
        }
 
+       of_get_mac_address(np, dev->dev_addr);
+       if (!is_valid_ether_addr(dev->dev_addr)) {
+               dev_err(&pdev->dev, "invalid MAC address, using random address\n");
+               eth_random_addr(dev->dev_addr);
+       }
+
        pr_info("%s: Atheros AG71xx at 0x%08lx, irq %d, mode: %s\n",
                dev->name, (unsigned long) ag->mac_base, dev->irq,
                phy_modes(ag->phy_if_mode));

no you should test the error code of of_get_mac_address

int ret;

ret = of_get_mac_address...
if (ret)
  return ret;

@ptpt52
Copy link
Contributor

@ptpt52 ptpt52 commented on 91a52f2 Dec 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the ret is ret = -2

Please sign in to comment.