netdev - Re: [PATCH] [v8] net: emac: emac gigabit ethernet controller driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 17 Aug 2016 22:05:51 +0200
From:	Andrew Lunn <andrew@...n.ch>
To:	Timur Tabi <timur@...eaurora.org>
Cc:	Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	sdharia@...eaurora.org, shankerd@...eaurora.org,
	vikrams@...eaurora.org, cov@...eaurora.org, gavidov@...eaurora.org,
	robh+dt@...nel.org, bjorn.andersson@...aro.org,
	mlangsdo@...hat.com, jcm@...hat.com, agross@...eaurora.org,
	davem@...emloft.net, LinoSanfilippo@....de
Subject: Re: [PATCH] [v8] net: emac: emac gigabit ethernet controller driver

> Something is odd about of_mdiobus_register().
> 
> This function scans child nodes to look for PHYs:
> 
> /* Loop over the child nodes and register a phy_device for each phy */
> for_each_available_child_of_node(np, child) {
> 	addr = of_mdio_parse_addr(&mdio->dev, child);
> 
> And in my driver, this works.  However, later when I try to call
> of_phy_find_device(), it fails.  That's because of_phy_find_device()
> wants the np of the child node.  But the only way to get that is
> with a phy-phandle property which I need to manually parse.

You do need the phy-handle, because you can have multiple PHYs, and
ethernet switches on the MDIO bus. You need to indicate which of those
PHYs is connected to this MAC.

> So what's the point of having of_mdiobus_register() parse child
> nodes, if you need a phy-phandle pointer anyway?

So we know about the other phys and switches on the MDIO bus, can load
the driver for them, etc. This is standard policy for a bus
driver. When you instantiate the bus, you enumerate it to see what is
connected to it. You do that either with of_mdiobus_register() or
mdiobus_register().

However, i do agree about having a helper do the phy-handle parsing
would be nice, and it should also handle fixed phys as well.

      Andrew