On 09/05/13 19:51, Marek Vasut wrote: >>> Why not wrap board_usb_init() and board_usb_init_fail() into single call. >>> You now pass some flags to board_usb_init() already, so just add another >>> for the fail case. How does it sound to you? >> >> Like overengineering. It would lead to "board_usb_init(USB_INIT_ALL, >> USB_INIT_DEVICE, USB_CLEANUP)" calls, which are not very readable. > > This is not what I mean, see this: > > int board_usb_init(int index, enum board_usb_init_type init) > > Add a new "init" type (or maybe change the init field to be flags) that will > say > "OK, do a fail init" ?
As for providing a way to do a selective cleanup if anything gone wrong, it's a good idea, but adding such functionality to board_usb_init() would be confusing, especially for newcomers. Why not do this in board_usb_init_fail(int index, enum board_usb_init_type)? ...and maybe rename board_usb_init_fail to board_usb_cleanup. >>> Moreover, the 'int index' should likely be unsigned int and the special >>> value to init all controllers at once should probably then be 0xffffffff >> >> Despite our greatest ambitions, I don't think we're likely to use more >> than 2^31-1 USB controllers at a time. Besides, negative values look >> better both in code and debugger session. > > Thinking of it further, instead of using negative value here, like I > mentioned > above, why not make the "board_usb_init_type" into a field of flags , then > add > flag to init all controllers at once ? That's unnecessary. It wouldn't lead to any practical advantage over existing interface. Best Regards, -- Mateusz Zalega Samsung R&D Institute Poland _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot