Hi Alex,
Am 2019-12-11 16:37, schrieb 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 <mich...@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.
are you sure? afaik it works by reading the /alias/ethernetN phandles
which gets ethNaddr assigned if you set the FDT_SEQ_MACADDR_FROM_ENV
config option. I've just tried it, here is my linux dts diff
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -17,6 +17,11 @@
#address-cells = <2>;
#size-cells = <2>;
+ aliases {
+ ethernet0 = &enetc_port0;
+ ethernet1 = &enetc_port1;
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
@@ -761,10 +766,12 @@
enetc_port0: ethernet@0,0 {
compatible = "fsl,enetc";
reg = <0x000000 0 0 0 0>;
+ local-mac-address = [ 00 00 00 00 00 00
];
};
enetc_port1: ethernet@0,1 {
compatible = "fsl,enetc";
reg = <0x000100 0 0 0 0>;
+ local-mac-address = [ 00 00 00 00 00 00
];
};
enetc_mdio_pf3: mdio@0,3 {
compatible = "fsl,enetc-mdio";
That way the mapping between ethNaddr and the network device can also
be changed by the user by changing the linux device tree.
also, uboot should respect the /aliases/ethernetN, too.
BTW what will be the source of the network addresses, the u-boot
environment variables? (which might be set either by the user or some
kind of board specific code).
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.
I see. But that shouldn't happen with the code above. But are you sure
that this
+ uclass_get(UCLASS_ETH, &uc);
+ uclass_foreach_dev(dev, uc) {
will work then? in my config (just enetc-0) there is only one eth device
# dm tree
[snip]
pci 2 [ + ] pci_generic_ecam |-- pcie@1f0000000
eth 0 [ + ] enetc_eth | |-- enetc-0
mdio 0 [ + ] enetc_mdio | |-- emdio-3
pci_generi 0 [ ] pci_generic_drv | |-- pci_4:0.4
dsa 0 [ ] felix-switch | |-- felix-switch
pci_generi 1 [ ] pci_generic_drv | `-- pci_4:1f.0
[snip]
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.
This will also only be called if the device is not disabled in u-boot,
from what I see. So it might be better to "fix" this.
I bet you are not the first/only one which needs this kind of "how do we
propagate the hardware address to linux" where the devices might be
unused
in u-boot. There must be a better way to do this ;)
-michael
