[PATCH v2] drivers: net: fsl_enetc: Pass on primary MAC address to Linux
Vladimir Oltean
olteanv at gmail.com
Wed Dec 11 13:46:15 CET 2019
Hi Michael,
On Wed, 11 Dec 2019 at 00:48, Michael Walle <michael at 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 at 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 at 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 at 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
More information about the U-Boot
mailing list