On Tue, 10 Dec 2019 at 17:03, Alexandru Marginean alexm.osslist@gmail.com wrote:

On 12/10/2019 3:54 PM, Vladimir Oltean wrote:

...

Hi Alex,

On Tue, 10 Dec 2019 at 16:21, Alex Marginean alexandru.marginean@nxp.com wrote:

...

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.

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.

board/freescale/ls1028a/ls1028a.c | 4 ++ drivers/net/fsl_enetc.c | 65 ++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/board/freescale/ls1028a/ls1028a.c b/board/freescale/ls1028a/ls1028a.c index 3977ecf896..fac03f55e9 100644 --- a/board/freescale/ls1028a/ls1028a.c +++ b/board/freescale/ls1028a/ls1028a.c @@ -150,6 +150,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 02c1ee70d9..f8fe7d4d8d 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};

•   u16 lower = *(const u16 *)(enetaddr + 4);
  

•   u32 upper = *(const u32 *)enetaddr;
  

•   if (ierb_fn_to_pf[devfn] < 0)
  

•           return;
  

•   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);
  

The Linux Documentation/devicetree/bindings/net/ethernet.txt says:

• local-mac-address: array of 6 bytes, specifies the MAC address that was assigned to the network device;
• mac-address: array of 6 bytes, specifies the MAC address that was last used by the boot program; should be used in cases where the MAC address assigned to the device by the boot program is different from the "local-mac-address" property;

I'm not sure which property should be used here, but I think local-mac-address.

U-Boot does use (well, if needed) that address, if that is of any importance. of_get_mac_address in Linux has this comment: /**

• Search the device tree for the best MAC address to use.

'mac-address' is

• checked first, because that is supposed to contain to "most recent" MAC
• address. If that isn't set, then 'local-mac-address' is checked next,
• because that is the default address. If that isn't set, then the

obsolete

• 'address' is checked, just in case we're using an old device tree.

If any

• of the above isn't set, then try to get MAC address from nvmem cell

named

• 'mac-address'.

I think mac-address is fine, we can leave local-mac-address to be used with some default in Linux DT, in case mac-address is missing. I don't have a strong opinion either way though.

And the generic fdt_fixup_ethernet function does set both mac-address and local-mac-address to the same value. Strange. And for what it's worth, the few boards that call the generic fdt_fixup_ethernet do it from ft_board_setup as well.

I don't have a strong opinion either. I was actually thinking the other way around: you want to override the U-Boot fixup (without recompiling it) with a value defined in the device tree. So U-Boot shouldn't set the highest-priority property (mac-address). But I don't have any use case in mind for that. So it's probably fine either way (not very consistent).

Thanks! Alex

...

...

•   }
  

+}

• /*
  • Bind the device:
  • • set a more explicit name on the interface

@@ -551,7 +614,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,
  

-- 2.17.1

Thanks, -Vladimir

Regards, -Vladimir