Hi Michael, On Wed, 11 Dec 2019 at 00:48, Michael Walle <[email protected]> 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 <[email protected]> > > --- > > > > 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 <[email protected]> > > + > > + 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
