[PATCH v2] drivers: net: fsl_enetc: Pass on primary MAC address to Linux

[Alexandru Marginean]

On 12/10/2019 11:47 PM, Michael Walle 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.

By proprietary do you mean IERB?

Network cards normally have a ROM which comes preset from the factory 
with default MAC addresses.  At run-time drivers can use the default 
address or replace it.  The MAC address in IERB is the default MAC 
address for ENETC interfaces at run-time, this address is available to 
the driver/stack after issuing an FLR and in the absence of a DT node 
for the PCI function.
We can pass MAC addresses to Linux through DT alone, but that's more of 
a hassle if Linux decides to assign the PCI function to a guest or 
user-space app.  Using the IERB values doesn't require any fix-up for 
guest DTs.

IERB is not actually a ROM though and we need U-Boot to set factory MAC 
addresses into IERB some time before Linux boots.

> Also, shouldn't write_hwaddr callback be implemented instead of the
> enetc_set_ierb_primary_mac()?

OK, makes sense, I will move the code there.

I'll send a v3,
Thanks!
Alex

> 
> -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.
> 
>> +
>> +    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 */