enclustra-bsp / xilinx-linux

Other
12 stars 15 forks source link

Setting eth1addr from U-Boot #9

Closed tsat-psv closed 5 years ago

tsat-psv commented 5 years ago

When using miltiple ethernet interfaces and trying to set a MAC in U-Boot for eth1 interface as per documantation (eth1addr parameter,) it seems not to be picked up by the kernel on boot and random MAC is assigned.

Any advice on how to resolve this?

tholzsche commented 5 years ago

Assign a valid address.

I have not been able to reproduce this problem (baseboard: PE1, module: xu5-5ev-2i, ebe_release: v1.8.2). Can you give us some more details about your environment (software/hardware/version ...).

tsat-psv commented 5 years ago

Thanks for the replay @tholzsche!

These are the assigned addresses used in U-Boot: ethaddr=20:B0:F7:04:0B:AA eth1addr=20:B0:F7:04:0B:BB

On boot I see this trace:

macb e000b000.ethernet eth0: Cadence GEM rev 0x00020118 at 0xe000b000 irq 30 (20:b0:f7:04:0b:aa)
macb e000c000.ethernet: invalid hw address, using random
macb e000c000.ethernet eth1: Cadence GEM rev 0x00020118 at 0xe000c000 irq 31 (1e:31:b1:97:b7:41)

I also tried adding the parameter explicitly in the bootargs, and see this printed in the boot log: Kernel command line: console=ttyPS0,115200 rw earlyprintk, ethaddr=20:B0:F7:04:0B:AA, eth1addr=20:B0:F7:04:0B:BB rootwait root=/dev/mmcblk0p2, so I'm somewhat confident the parameter is passed to the kernel.

The hardware we are using is your Mars ZX3 with our propiratary host board. We are on the latest ebe 1.8.2.

The second ethernet is defined in the devicetree like so (and works fine in linux):

+   mdio@e000b000 {
+       compatible = "cdns,macb-mdio";
+       reg = <0xe000b000 0x1000>;
+       clocks = <&clkc 30>, <&clkc 30>, <&clkc 13>;
+       clock-names = "pclk", "hclk", "tx_clk";
+       #address-cells = <1>;
+       #size-cells = <0>;
+       phy3: ethernet-phy@3 {
+               reg = <3>;
+       };
...
+   mdio1@e000c000 {
+          compatible = "cdns,macb-mdio";
+          reg = <0xe000c000 0x1000>;
+          clocks = <&clkc 31>, <&clkc 31>;
+       clock-names = "pclk", "hclk";
+          #address-cells = <1>;
+          #size-cells = <0>;
+       phy7: ethernet-phy@7 {
+            reg = <7>;
+       };
+   };
...
+&gem1 {
+   status = "okay";
+   phy-mode = "mii-id";
+   phy-handle = <&phy7>;
+   xlnx,emio = <1>;
+};
tsat-psv commented 5 years ago

I think I have found something that might cause this, and I'll try to patch it and see if it helps.

According to this old patch, there was a bug in the drivers that cuased this earlier. https://github.com/Xilinx/linux-xlnx/commit/467c0558bff78cdf3daa67a73a3bd16b60d0d930

When I check the source of the driver that seems to be used for my nic, /xilinx-linux/drivers/net/ethernet/cadence/macb.c is seems to have this memcopy bug:

        if (is_valid_ether_addr(addr)) {
            memcpy(bp->dev->dev_addr, addr, sizeof(addr));
            return;
        }
    }

    dev_info(&bp->pdev->dev, "invalid hw address, using random\n");
    eth_hw_addr_random(bp->dev);

I'll update here once I know more. I appriciate the quick response to my initial post.

tholzsche commented 5 years ago

The driver worked for me ... . Did you add an alias?

...
    aliases {
+       ethernet1 = &gem1;
...
tsat-psv commented 5 years ago

Do you mean in the U-Boot or linux dts?

tsat-psv commented 5 years ago

I added the alias in he U-Boot sources for the Mars-ZX3 dts, but it did not make any difference. I cant see the alias being used anywhere either? I'll try to apply the patch mentioned earlier to the driver, to see if it makes any difference.

tholzsche commented 5 years ago

Do you mean in the U-Boot or linux dts?

Linux.

tsat-psv commented 5 years ago

Ok, there are no aliases there from before, but I can try adding it. Do you have a reference to where this alias would be picked up and used in this context?

tsat-psv commented 5 years ago

@tholzsche, I added the aliases that are present in the ZX3 dts in the U-Boot sources to the Linux sources, and indeed, as you said, the MAC is now picked up! Thanks alot for the help. Shall I open a PR to add these aliases to this https://github.com/enclustra-bsp/xilinx-linux repo so that others won't struggle in the future?

tholzsche commented 5 years ago

Atm this solution is solely for your base board. I do not think that a PR is needed.