Re: [PATCH 18/43] dt-bindings: net: Add DT bindings ep93xx eth

Hello Andrew!

On Mon, 2023-04-24 at 15:39 +0200, Andrew Lunn wrote:

quoted

+  copy_addr:
+    type: boolean
+    description:
+      Flag indicating that the MAC address should be copied
+      from the IndAd registers (as programmed by the bootloader)

Looking at ep93xx_register_eth(), all callers are setting copy_addr
to
1. So i don't think you need this.

Agreed. Dropped copy_addr entirely.

quoted

+
+  phy_id:
+    description: MII phy_id to use

The eEP93xx Ethernet driver is a very old driver, so it is doing MDIO
and PHY the old way. Ideally you should be using ep93xx_mdio_read()
and ep93xx_mdio_write() to create an MDIO bus with
of_mdiobus_regsiter, and then use a phy-handle to point to the PHY on
the bus. It will then be the same as all other ethernet drivers using
DT.

I've tinkered with the preferred way, however this involves turning on 

- CONFIG_PHYLIB
- CONFIG_MDIO_DEVICE

And maybe CONFIG_MICREL_PHY (at least for me, unless i can use some
common phy driver) which implies a kernel size increase - which is
undesirable for us.

Can we slip by with something like:

+       np = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
+       if (!np) {
+               dev_err(&pdev->dev, "Please provide \"phy-handle\"\n");
+               return -ENODEV;
+       }
+
+       if (of_property_read_u32(np, "reg", &phy_id)) {
+               dev_err(&pdev->dev, "Failed to locate \"phy_id\"\n");
+               return -ENOENT;
+       }

And standard device tree bindings ?:

+    ethernet@80010000 {
+      compatible = "cirrus,ep9301-eth";
+      reg = <0x80010000 0x10000>;
+      interrupt-parent = <&vic1>;
+      interrupts = <7>;
+      phy-handle = <&phy0>;
+      mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        phy0: ethernet-phy@1 {
+          reg = <1>;
+          device_type = "ethernet-phy";
+        };
+      };
+    };

    Andrew