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

[Alexandru Marginean]

On 12/11/2019 2:16 PM, Michael Walle wrote:
> Hi Vladimir,
> 
> Am 2019-12-11 13:46, schrieb Vladimir Oltean:
>> 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.
> 
> yes, but u-boot already sets the mac-address/local-mac-address property
> in the device tree already in a generic way, see fdt_fixup_ethernet().

I think fdt_fixup_ethernet is not a good choice for us.
The issue is that it ties Linux DT to device indexes in U-Boot.  That's 
a problem if we plug an Eth PCI card in, we would need to change Linux 
DT, which is definitely not desirable.
We actually do use PCI Eth cards with some of our boards and U-Boot does 
pick those up and indexes shift.

Also U-Boot and Linux DTs have to be in sync all the time, if we disable 
one port in U-Boot but not in Linux we would mix up MAC addresses.

So as far as using generic fix-up code I'm all for it, but in this case 
we would need some platform specific rules to match Linux DT nodes to 
U-Boot eth addresses.

>> 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.
> 
> according to the comment in eth-uclass.c this is not for (u-boot) runtime:
> 
>   /*
>    * Devices need to write the hwaddr even if not started so that Linux
>    * will have access to the hwaddr that u-boot stored for the device.
>    * This is accomplished by attempting to probe each device and calling
>    * their write_hwaddr() operation.
>    */
> 
> and the runtime mac address for u-boot is already set enetc_start().
> 
> -michael

This is fine, I'll move the IERB code to write_hwaddr.

Alex