30.03.2015 19:06, Florian Fainelli пишет: > 2015-03-30 7:39 GMT-07:00 Stas Sergeev <stsp@xxxxxxx>: >> 27.03.2015 20:15, Florian Fainelli пишет: >>> I think your concerns are valid, but I don't think there is going to be >>> any problem with the approach I suggested because there is a contract >>> that the fixed PHYs and regular PHYs need to >> Hello Florian. >> >> As promised, today I tried to resurrect my first implementation >> and do things as you suggested: install the link_update callback >> for mvneta privately. >> I feel SGMII setup is very common and deserves the separate API, >> not the per-driver handling, but in any case, I'd like to show >> the implementation first, then discuss. >> >> Unfortunately, it didn't work quite right. >> The problem is that mvneta calls phy_disconnect() on .ndo_stop >> callback. After that, phy->attached_dev becomes NULL, and so the >> link_update callback gets called with net_dev==NULL! And crashs. >> Of course I can easily work around that, but IMHO its a bug - >> the one that actually gets fixed by the patches I posted previously. > > I actually submitted some patches a while ago that allow you to > unregister the fixed_link_update callback before in case you need to, > precisely for that. Since this is specific to dealing with a fixed > PHY, it is the driver responsibility to know that is has registered a > fixed_link_update callback and then unregister it by passing a NULL > callback as the new callback. Today I posted the patch that does exactly that. But I already see the problem: --- [ 10.535424] mvneta f1030000.ethernet eth1: Link is Up - Unsupported (update phy.c)/Half - flow control off --- Seems like if I register a callback after of_phy_connect() (and unregister before disconnect), there is a small window between connect and setting the callback. If someone at that time will query phy, he'll get an undefined status (speed). And before of_phy_connect() the phy_device pointer is not known to register callback earlier. Of course I can see the possible work-arounds, but I wonder if something better can be done than using of_phy_find_device() and related magic right before of_phy_connect() just for that. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
