Hi Andy, On Wed, Aug 22, 2012 at 3:40 PM, Andy Fleming <[email protected]> wrote: > On Wed, Aug 22, 2012 at 2:59 PM, Troy Kisky > <[email protected]> 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 _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
