From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joe Hershberger Date: Wed, 22 Aug 2012 15:50:11 -0500 Subject: [U-Boot] [PATCH V1 1/3] phy: add phy_connect_by_mask In-Reply-To: References: <1345502918-18305-1-git-send-email-troy.kisky@boundarydevices.com> <5032C349.3090507@boundarydevices.com> <50353A24.8030601@boundarydevices.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Andy, On Wed, Aug 22, 2012 at 3:40 PM, Andy Fleming wrote: > On Wed, Aug 22, 2012 at 2:59 PM, Troy Kisky > wrote: >> On 8/20/2012 5:35 PM, Andy Fleming wrote: >> The same way it works currently. I removed no features. > > > Agreed. But the way it works currently is bad. Many drivers do this: > > include/configs/board.h: > #define MY_PHY_ADDR x > > drivers/net/myenet.c: > > ... > phydev = phy_connect(blah, blah, MY_PHY_ADDR); > > This is a bad idea, because it encodes the implicit assumptions that: > > 1) There's only one PHY in the whole system > 2) The PHY address can be known at compile time > > Later, when someone adds a second ethernet controller, a frequent > solution is then to make a MY_PHY_ADDR1, MY_PHY_ADDR2, and then add > some logic to the driver to pick which one to use. In general, this > makes the driver brittle, as adding and rearranging controllers is > fairly easy for logic designers, who don't have to care whether their > new logic will continue to operate the old chip. > > Alternatively, when someone causes the PHY address to vary such that > assumption #2 is violated, it's not uncommon to solve it by searching > the bus. But this further entrenches assumption #1. > > Just to underscore this, I'm currently working on a product with 2 MACs and 2 PHYs... both PHYs share the MDIO bus of the first MAC. It's convenient for hardware since they only have to use up pins for one MDIO bus. I definitely want to get to a point where supporting this sort of topology is cleaner and easier. >> So, should I fix something before this patch? > > My thought is that your solution is quite elegant, but further > entrenches the assumption that there will be only one ethernet > controller. In my mind, the best way to solve this is: > > 1) Modify the driver so that the PHY address is passed in from board > initialization code programmatically. As a nod to the effort of doing > so for all boards, you can create a default value (ie - as it was), > that can be overridden by board code. > 2) Modify the search function to look for a valid PHY for a given > mask, and return the address of that PHY > 3) Add code to the board file which passes in the mask to the search > function, and then passes the resulting PHY address to the driver. I think this sounds good. -Joe