On Thu, Apr 21, 2011 at 4:38 PM, Wolfgang Denk <w...@denx.de> wrote: > Dear Simon Glass, > > In message <banlktikgucjpun2rhs2t2nyq4_kb9gk...@mail.gmail.com> you wrote: >> >> >> + eth = &usb_eth[usb_max_eth_dev].eth_dev; >> > >> > Index for eth is usb_max_eth_dev. >> > >> >> @@ -100,7 +102,10 @@ static void probe_valid_drivers(struct usb_device *> >> >> dev) >> >> * call since eth_current_> changed (internally >> >> called) >> >> * relies on it >> >> */ >> >> - eth_register(&usb_eth[usb_max_eth_dev - 1].eth_dev); >> >> + eth_register(eth); >> > >> > You change the behaviour here. Please confirmt his is really >> > intentional. >> >> Yes. Since I am using an 'eth' pointer I don't need to index the array >> again. The behaviour is the same as before. > > No, it is not. Before, we were accessing entry N-1 here. Now we use > entry N. usb_max_eth_dev != usb_max_eth_dev - 1
Hi Wolfgang, Hmmm. If you see the usb_max_eth_dev++ below, it is increasing the index variable to keep eth_register() happy. I have kept that behaviour. So let's say usb_max_eth_dev is 3. The sequence is: - set eth to point to item 3 - increase usb_max_eth_dev to 4 as required by eth_register() - call eth_register() with item 3 (i.e. eth pointer hasn't changed) - call eth_write_hwaddr with item 3 Maybe look at the whole code with my patch applied? Let me know if I am missing something. + eth)) { /* found proper driver */ /* register with networking stack */ usb_max_eth_dev++; @@ -100,7 +102,10 @@ static void probe_valid_drivers(struct usb_device *dev) * call since eth_current_changed (internally called) * relies on it */ - eth_register(&usb_eth[usb_max_eth_dev - 1].eth_dev); + eth_register(eth); + if (eth_write_hwaddr(eth, "usbeth", + usb_max_eth_dev - 1)) + puts("Warning: failed to set MAC address\n"); break; } > >> >> + * base_name - base name for device (NULL for "eth") >> > >> > This is an atitifical decision for the API which is difficult to >> > understand. It just makes the code and understanding it more >> > difficult. Just pass "eth" when you mean it. >> >> The intention was to avoid everyone having to pass the correct value - >> potential for error, etc. I could have created a #define, but decided >> on this. > > Ummm... but having everyone to pass the correct value is actually a > really good thing to have! OK fair enough, it is in there now. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot