Hi Joe, On 16 February 2015 at 21:17, Joe Hershberger <joe.hershber...@gmail.com> wrote: > Hi Simon, > > > On Sun, Feb 15, 2015 at 9:59 AM, Simon Glass <s...@chromium.org> wrote: >> >> Hi Joe, >> >> On 13 February 2015 at 19:33, Joe Hershberger <joe.hershber...@gmail.com> >> wrote: >> > On Thu, Feb 12, 2015 at 11:14 PM, Simon Glass <s...@chromium.org> wrote: >> >> >> >> Hi Joe, >> >> >> >> On 10 February 2015 at 23:08, Joe Hershberger >> >> <joe.hershber...@gmail.com> >> >> wrote: >> >> > Hi Simon, >> >> > >> >> > On Tue, Feb 10, 2015 at 10:39 PM, Simon Glass <s...@chromium.org> >> >> > wrote: >> >> >> >> >> >> Hi Joe, >> >> >> >> >> >> On 10 February 2015 at 18:30, Joe Hershberger >> >> >> <joe.hershber...@ni.com> >> >> >> wrote: >> >> >> > Before this patch, if the sequence numbers were resolved before >> >> >> > probe, >> >> >> > this code would insist on defining new non-conflicting-with-itself >> >> >> > seq >> >> >> > numbers. Now any "non -1" seq number is accepted as already >> >> >> > resolved. >> >> >> >> >> >> Can you explain what problem this solves? At present, when probing a >> >> >> device, ->seq must be -1 (sort-of by definition since it doesn't >> >> >> exist >> >> >> as an active device in the uclass). >> >> > >> >> > Please look at eth_post_bind() in patch 07/14. The Ethernet devices >> >> > need to >> >> > write their hardware addresses to the registers in bind (since it >> >> > needs >> >> > to >> >> > happen regardless of the device being used so that Linux will see the >> >> > MAC >> >> > address). As such, the sequence number is needed to look up the >> >> > ethaddr. In >> >> > order to avoid probing all the devices to get the seq number >> >> > resolved, I >> >> > resolve it in post_bind to avoid the rest of the overhead (thus no >> >> > longer >> >> > probing in post_bind, which was one of the issues previously). Then >> >> > when >> >> > probe comes along, the seq is already resolved. That's why this >> >> > patch >> >> > is >> >> > needed. >> >> >> >> OK I see. >> >> >> >> This is a bit messy. If the MAC address assignment is part of the bind >> >> step then it shouldn't need the seq number. >> > >> > Not sure why you say that. The reason I need the seq number is because >> > I >> > need to look up the proper env variable for the MAC address. E.g. >> > ethaddr, >> > eth2addr, etc. The seq number select which one to read from the env. >> >> We should be able to do this after a probe. A device which is bound >> but not probed does not have a sequence number, as things currently >> stand. >> >> > >> >> I can think of some poor ways to do this but a nice way is not obvious! >> > >> > Not sure what you're referring to here. What is "this" in this context? >> >> Figuring out the sequence number. >> >> > >> >> One option would be probe all the Ethernet devices on startup. If >> >> probe() only set up the hardware (including MAC address) then that >> >> might work. It would be fairly fast since it wouldn't involve starting >> >> up the link, etc. I suspect you are worried about a lot of Ethernet >> >> devices sitting around probed by unused. I'm not sure if that matters >> >> though. >> > >> > I had it probing the devices originally (by calling first and next) and >> > you >> > commented that it shouldn't happen until the devices are used. However, >> > I >> >> That was because your code was probing things in the bind mehod. >> >> > don't think we can guarantee that all drivers that come later will have >> > simple probe (since that's not part of the contract). I think I agree >> > with >> > your original statement that we should not probe. It seems more >> > suitable to >> > write the hwaddr in bind as a known and limited side effect. >> >> I don't like the idea of an ethernet device supporting writing its >> hardware address before it is probed. Until it is probed we don't >> really know it is there, nor where it is exactly (bus, memory >> address). So I think writing the hardware address makes more sense >> after probe. But probe should not happen as part of bind. It seems to >> me it could happen in your eth_init(). > > OK. I can see why you prefer not to have the hardware accessed in bind. In > order to probe the devices (but not from bind) I'll have to add back > eth_initialize() and have it probe all of the devices. the "eth_init()" > that you see here is what gets called when the network is enabled, so it > certainly shouldn't go in eth_init(). I could potentially rename it to > eth_start() or something. That would be clearer, but would break from the > former naming for anyone familiar.
I see. Maybe eth_probe_all_devices()? > >> > The seq number resolution seems fairly well contained as I implemented >> > it in >> > bind. I simply call the core function and write the result to the >> > device >> > member. Then of course this patch to remove the assert. >> >> Yes it is well contained, but I still don't think it is right. If you >> want to put '#ifndef CONFIG_DM_NET' around the assert in >> uclass_resolve_seq() while we work it out, that is OK with me. > > I will work on making it correct instead of adding that. OK. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
