Re: [PATCH] [v9] net: emac: emac gigabit ethernet controller driver

Rob Herring wrote:

quoted

+		internal-phy = <&emac_sgmii>;

Can't this use the standard generic phy binding (i.e. 'phys'). It's a
bit confusing as there's the ethernet phy binding (phy-handle) and the
generic one.

It's not a generic phy.  It's a funky "internal phy" that differs among 
SOCs.  I call it the internal phy, but I could use another name. 
Internally, some people call it the "sgmii phy", but I don't think 
that's accurate.

I can call it "emac-phy", but I don't know if that's any better.

quoted

+		phy-handle = <&phy0>;

This is bit redundant as the phy is the child node. I guess if you had
multiple devices on the mdio bus you would need it. I'd drop it if you
don't envision needing it and the kernel doesn't require it.

That's what I thought to, but without it, of_phy_find_device() won't 
work.  I need a pointer to the phy node, and I use of_parse_phandle() to 
get it:

	struct device_node *phy_np;

	ret = of_mdiobus_register(mii_bus, np);
	if (ret) {
		dev_err(&pdev->dev, "could not register mdio bus\n");
		return ret;
	}

	phy_np = of_parse_phandle(np, "phy-handle", 0);
	adpt->phydev = of_phy_find_device(phy_np);

quoted

+
+		#address-cells = <1>;
+		#size-cells = <0>;
+		phy0: ethernet-phy@0 {

It's just an example, but don't we require compatible strings for phys
now?

Nope. I had a compatible property, but it broke 
of_mdiobus_child_is_phy().  I don't want to specify why kind of phy it 
is.  I want to let phylib figure it out.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html