Hello Ben,
I am trying to port u-boot on a system which deploys two different types of ethernet PHYs, and don't see a good way of doing it with the current sw structure.
There are concurrent implementations for supporting different PHYs, and it looks like quite often bringing in a new etherent controller brings in a duplicate PHY driver code. OTOH it is always presumed that the same kind of phy is used on all ports.
I want to modify this scheme, but I don't want to go too far on a tangent, as we here plan to integrate our changes back into the master repository one of those days.
Here is what I suggest doing:
we introduce a concept of 'phy bus' - an MDIO bus for instance, and 'phy interface' which is the phy driver.
For illustration purpose, the phy bus could look like this: struct phy_bus { int (*write)(u8 addr, u16 reg, u16 value); int (*read)(u8 addr, u16 reg, u16 *value); }
and the driver would provide methods like
struct phy_driver { int (*reset)(struct phy_bus*, u8 addr)) int (*init)(struct phy_bus*, u8 addr) ... }
the set of methods could be fine tuned at any time obviously, the important thing is that each method would receive the pointer to the phy bus to be used to communicate with the device.
Then, in the ethernet device structure we would add two pointers: to the bus serving the port and to the driver serving the port.
This would allow any new ethernet controller provide its own mdio bus implementation and then use existing phy driver the same way any other controller uses it. This would also allow to attach different phy devices to different ethernet ports. And of course the multiple PHY implementations won't be needed anymore, just one instance will serve any ethernet device.
What do you think?
TIA, Vadim
On Thu, Sep 25, 2008 at 11:09 PM, Ben Warren biggerbadderben@gmail.com wrote:
Wolfgang Denk wrote:
Dear Ben,
I just ran over this piece of code in NetLoop() [see "net/net.c"]:
286 int 287 NetLoop(proto_t protocol) 288 { ... 322 eth_halt(); 323 #ifdef CONFIG_NET_MULTI 324 eth_set_current(); 325 #endif 326 if (eth_init(bd) < 0) { 327 eth_halt(); 328 return(-1); 329 }
Am I reading this correctly that we eth_halt() and eth_init() the network interface for each and every call to NetLoop?
Yes, it looks that way. Ripe for gutting.
This looks terribly inefficient to me - is there any rationale behind this?
Probably, but it escapes me. It most certainly predates my involvement in this project.
Also, the eth_set_current() checking should IMHO be done just once, before we start a network transfer, or when we actually switch interfaces, but not for each and every call to NetLoop() ?
Maybe, but eth_set_current() is pretty lightweight. NetLoop is only called when we start a network transfer, so this doesn't seem too egregious. It could definitely stand to be refactored.
Best regards,
Wolfgang Denk
regards, Ben _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot