Hi Michael,
On Wed, 11 Dec 2019 at 00:48, Michael Walle michael@walle.cc wrote:
Am 2019-12-10 15:55, schrieb Alex Marginean:
Passes on the primary address used by u-boot to Linux. The code does a DT fix-up for ENETC PFs and sets the primary MAC address in IERB. The address in IERB is restored on ENETC PCI functions at FLR.
I don't get the reason why this is done in a proprietary way. What is the difference between any other network interface whose hardware address is set up in the generic u-boot code.
Also, shouldn't write_hwaddr callback be implemented instead of the enetc_set_ierb_primary_mac()?
At the moment, the Linux driver ignored the device tree and only reads the POR values of the SIPMAR registers. The reset value of those comes from the IERB space, which U-Boot is configuring. So it would be good if that behavior keeps working.
It would also be good if the Linux driver called of_get_mac_address, so it needs the device tree binding for that. That's why both fixups are performed, and why the generic function is not used.
As for the write_hwaddr callback instead of enetc_set_primary_mac_addr, that is valid but I suppose it is outside the scope of this patch. That function is related to the runtime MAC address and not to the MAC passed to Linux.
-michael
Signed-off-by: Alex Marginean alexandru.marginean@nxp.com
The code is called fom ft_board_setup in board/freescale/ls1028a/ls1028a.c mostly for consistency with other LS parts. I'm open to suggestions though.
Changes in v2:
- fixed warning for fdt_fixup_enetc_mac being implicitly declared
board/freescale/ls1028a/ls1028a.c | 5 +++ drivers/net/fsl_enetc.c | 65 ++++++++++++++++++++++++++++++- drivers/net/fsl_enetc.h | 3 ++ 3 files changed, 72 insertions(+), 1 deletion(-)
diff --git a/board/freescale/ls1028a/ls1028a.c b/board/freescale/ls1028a/ls1028a.c index a9606b8865..1a82c95402 100644 --- a/board/freescale/ls1028a/ls1028a.c +++ b/board/freescale/ls1028a/ls1028a.c @@ -25,6 +25,7 @@ #include <fdtdec.h> #include <miiphy.h> #include "../common/qixis.h" +#include "../drivers/net/fsl_enetc.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -150,6 +151,10 @@ int ft_board_setup(void *blob, bd_t *bd)
fdt_fixup_icid(blob);+#ifdef CONFIG_FSL_ENETC
fdt_fixup_enetc_mac(blob);+#endif
return 0;} #endif diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c index 0ca7e838a8..3c043888db 100644 --- a/drivers/net/fsl_enetc.c +++ b/drivers/net/fsl_enetc.c @@ -14,6 +14,69 @@
#include "fsl_enetc.h"
+#define ENETC_DRIVER_NAME "enetc_eth"
+/*
- sets the MAC address in IERB registers, this setting is persistent
and
- carried over to Linux.
- */
+static void enetc_set_ierb_primary_mac(struct udevice *dev, int devfn,
const u8 *enetaddr)+{ +#ifdef CONFIG_ARCH_LS1028A +/*
- LS1028A is the only part with IERB at this time and there are
plans to change
- its structure, keep this LS1028A specific for now
- */
+#define IERB_BASE 0x1f0800000ULL +#define IERB_PFMAC(pf, vf, n) (IERB_BASE + 0x8000 + (pf) * 0x100 + (vf) * 8 \
+ (n) * 4)+static int ierb_fn_to_pf[] = {0, 1, 2, -1, -1, -1, 3};
wrong indendation
u16 lower = *(const u16 *)(enetaddr + 4);u32 upper = *(const u32 *)enetaddr;if (ierb_fn_to_pf[devfn] < 0)return;it would be easier to read if ierb_fn_to_pf[devfn] would be assigned to a local variable.
Alex, after you address this feedback from Michael, you can add my:
Reviewed-by: Vladimir Oltean vladimir.oltean@nxp.com
out_le32(IERB_PFMAC(ierb_fn_to_pf[devfn], 0, 0), upper);out_le32(IERB_PFMAC(ierb_fn_to_pf[devfn], 0, 1), (u32)lower);+#endif +}
+/* sets up primary MAC addresses in DT/IERB */ +void fdt_fixup_enetc_mac(void *blob) +{
struct pci_child_platdata *ppdata;struct eth_pdata *pdata;struct udevice *dev;struct uclass *uc;char path[256];int offset;int devfn;uclass_get(UCLASS_ETH, &uc);uclass_foreach_dev(dev, uc) {if (!dev->driver || !dev->driver->name ||strcmp(dev->driver->name, ENETC_DRIVER_NAME))continue;pdata = dev_get_platdata(dev);ppdata = dev_get_parent_platdata(dev);devfn = PCI_FUNC(ppdata->devfn);enetc_set_ierb_primary_mac(dev, devfn, pdata->enetaddr);snprintf(path, 256, "/soc/pcie@1f0000000/ethernet@%x,%x",PCI_DEV(ppdata->devfn), PCI_FUNC(ppdata->devfn));offset = fdt_path_offset(blob, path);if (offset < 0)continue;fdt_setprop(blob, offset, "mac-address", pdata->enetaddr, 6);}+}
/*
- Bind the device:
- set a more explicit name on the interface
@@ -583,7 +646,7 @@ static const struct eth_ops enetc_ops = { };
U_BOOT_DRIVER(eth_enetc) = {
.name = "enetc_eth",
.name = ENETC_DRIVER_NAME, .id = UCLASS_ETH, .bind = enetc_bind, .probe = enetc_probe,diff --git a/drivers/net/fsl_enetc.h b/drivers/net/fsl_enetc.h index 0bb4cdff47..e441891468 100644 --- a/drivers/net/fsl_enetc.h +++ b/drivers/net/fsl_enetc.h @@ -226,4 +226,7 @@ int enetc_mdio_read_priv(struct enetc_mdio_priv *priv, int addr, int devad, int enetc_mdio_write_priv(struct enetc_mdio_priv *priv, int addr, int devad, int reg, u16 val);
+/* sets up primary MAC addresses in DT/IERB */ +void fdt_fixup_enetc_mac(void *blob);
#endif /* _ENETC_H */
Regards, -Vladimir